e238a71bbc567af083a4d481f5a6a99f51dcaa07
3000 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Joseph Doherty
|
e238a71bbc | docs(mtconnect): live gate PASSED against the shared AllowPut fixture (15/15); rig OPC UA leg deferred to the chrome session | ||
|
Joseph Doherty
|
0e3381d599 | merge: MTConnect write-back (IWritable via agent AllowPut observation writes); stale MQTT P3/browse-gap records reconciled | ||
|
Joseph Doherty
|
f13a89dbc3 | docs(mtconnect): final-review nits — two stale read-only comments; task statuses | ||
|
Joseph Doherty
|
b4480bf45a |
test(mtconnect): live write legs against the AllowPut fixture
The fixture Agent flips to AllowPut = true (its comment rewritten: writes are in
scope now that the driver implements IWritable) and the suite grows three
env-gated write legs, all still Assert.Skip-clean with no Agent reachable.
Devices.xml gains a DataItem, and that is the deliberate part. The write target
has to be one the SHDR adapter NEVER feeds, or the assertion races the stream --
but it must NOT be the existing never-fed fixture_block: the first successful
write takes an item off UNAVAILABLE for the life of the container, which would
silently rob Read_maps_an_unavailable_observation_to_BadNoCommunication of its
subject. So fixture_writeback (free-text EVENT, PROGRAM_COMMENT) is a separate,
dedicated target, overridable via MTCONNECT_WRITE_TARGET for a run pointed at a
real machine tool.
The three legs:
* Round trip -- driver write accepted, then the injected value read back off
/current over plain HTTP (not through the parser under test) AND through the
driver's own ReadAsync. The value carries a TickCount64 suffix: the Agent
keeps an injected observation for the container's life, so a constant would
be satisfied by a previous run's leftover.
* Agent refusal -> BadCommunicationError. The design doc doubted this leg was
constructible ("an id in the probe but refused by the agent"); it is, and the
mechanism is BuildWriteTargets. Write targets are derived from the tags the
BINDING serves, with the device resolved from the probe model and falling
back to the configured deviceName -- so an authored tag naming an id the
Agent never declared, on a driver whose deviceName IS set, resolves to a real
URL and is POSTed. That is exactly the shape a stale tag has after a DataItem
is renamed. The second half of the test is the falsifiability control: the
SAME unknown id with NO deviceName has no URL and must be refused locally as
BadNodeIdUnknown. Two different codes is what proves the first write actually
reached the Agent rather than being short-circuited into a look-alike.
* Disabled gate -- write.enabled absent returns BadNotWritable AND /current is
unchanged from the snapshot taken just before, which is what distinguishes
"refused" from "reported refused and sent anyway".
RAN LIVE, not just compiled: the fixture was brought up locally (OrbStack,
MTCONNECT_AGENT_HOST_PORT=5555 -- macOS AirPlay squats :5000) and the whole
suite passed 15/15, 0 skipped. Evidence beyond the green: the Agent reports
<ProgramComment dataItemId="fixture_writeback">OTOPCUA-WRITE-2CBCB530</...> and
its log carries "Error processing request:
/OtFixtureCnc?otopcua_no_such_data_item_2CBCB3D9=refuse-me" -- the refusal leg
demonstrably went to the wire. Offline (no Agent) the suite is 15 skipped, 0
failed; build 0 warnings / 0 errors.
Note for anyone reading agent.cfg: BufferSize = 14 is an EXPONENT (2^14 = 16384
observations), not a 14-deep ring, and /current is answered from the Agent's
per-DataItem checkpoint anyway -- so eviction is not a hazard for a write
read-back even on this busy fixture stream.
LiveOptions/ConfigJson/LiveDriverAsync gained writeEnabled + deviceName
parameters, both defaulted off/null so every pre-existing test still runs
against the shipped read-only driver whatever the Agent's AllowPut says. The
config JSON sources write.enabled FROM the options object, so the parsed config
can never disagree with the options a test thinks it built.
|
||
|
Joseph Doherty
|
203000a80a | docs: MTConnect write-back recorded; stale MQTT write-through/browse-gap records reconciled | ||
|
Joseph Doherty
|
b833dda673 |
fix(mtconnect): a throwing value formatter cannot escape the write batch
TryFormatValue's catch filter named only FormatException / InvalidCastException / OverflowException, and the call sits OUTSIDE WriteAsync's per-slot try. But Convert.ToString falls back to the value's OWN ToString/IFormattable - arbitrary caller code that may throw anything - so any other exception escaped WriteAsync uncaught and failed the entire batch, breaking the documented never-throws per-slot contract. Broadened to a catch-all; the slot gets BadTypeMismatch and the exception is logged at Warning (TryFormatValue became an instance method to reach _logger). One unformattable value now costs its own slot and nothing more. Falsification checked, not assumed: restoring the narrow filter makes the new test fail with a raw "System.InvalidOperationException : this formatter is broken" propagating out of WriteAsync - i.e. it fails for the contract-breach reason, not on an assertion. Two tests added: - a value whose ToString throws InvalidOperationException is refused BadTypeMismatch while the *following* good slot still writes (the load-bearing half - it proves the batch survived). - mid-batch cancellation: cancellation injected from inside slot 1's own formatting, so slot 0 has provably already reached the Agent. It keeps its real Good outcome while slots 1-2 fill BadCommunicationError. Reporting a landed write as failed is worse than reporting a failure, and a fill that started at 0 instead of i would do exactly that. Tests: 534 passed / 0 failed (was 532). Solution build: 0 errors, no warnings in any MTConnect project. |
||
|
Joseph Doherty
|
b29623cf99 |
feat(mtconnect): IWritable via agent observation writes
MTConnectDriver now implements IWritable over the Task-1 agent-client seam (WriteObservationAsync -> the Agent's AllowPut POST surface). A write is OBSERVATION INJECTION, not machine command: it sets what the Agent reports for a DataItem, is visible to every consumer of that Agent, and is overwritten by the next adapter/SHDR update. Off at two independent gates, both off by default - the Agent's own AllowPut and the driver's write.enabled (Task 0). Per-slot refusal order (never throws, strict N-in/N-out batch shape): write disabled -> BadNotWritable no live client -> BadNotConnected (ReadAsync's existing convention) reference unresolved -> BadNodeIdUnknown no resolvable device -> BadNodeIdUnknown CONDITION category -> BadNotWritable null / non-scalar value -> BadTypeMismatch agent refusal OR throw -> BadCommunicationError OperationCanceledException fills the remaining slots and returns. The Agent's refusal text is logged at Warning and never enters DriverHealth.LastError (type-only discipline); a refused write leaves health untouched. Device-name resolution: TagBinding gains WriteTargetsByDataItemId, built in BuildBinding from a probe-model walk that remembers which device each DataItem was declared under (name, or id when nameless) plus its category. Falls back to options.DeviceName; unresolvable -> BadNodeIdUnknown rather than a guess. Values render via Convert.ToString(..., InvariantCulture) - so a Boolean writes the invariant string "True"/"False", NOT "1"/"0". Arrays are refused rather than rendered: Convert.ToString would emit the literal "System.Double[]", which the Agent stores happily and which is invisible until read-back. FINDING - the CurrentWrite bit does NOT key on SecurityClassification, and the discovery stamping below is therefore cosmetic. Traced end to end: a browsed leaf's SecurityClass survives into the browse session's AttributeInfo (CapturedTreeBrowseSession.cs:53) and stops there - RawBrowseModal never reads it, RawBrowseCommitMapper.MapLeaf hardcodes TagAccessLevel.Read, and Tag has no SecurityClass column. AddressSpaceComposer:746/:809 sets Writable purely from Tag.AccessLevel == ReadWrite; AddressSpaceApplier ANDs !IsArray; OtOpcUaNodeManager.ComposeAccessLevel:2002 composes CurrentWrite from that alone. Driver capability is never consulted at compose time either - IWritable is checked only at runtime in DriverInstanceActor:755. Consequences: (a) raw tags already committed keep their authored AccessLevel, so nothing this commit does makes an existing node writable; (b) an operator must still set the tag to ReadWrite for an MTConnect write to be reachable over OPC UA; (c) conversely a ReadWrite tag on a non-IWritable driver has always advertised CurrentWrite and failed only at the actor - pre-existing, not touched here. Discovery stamps CanWrite(writeEnabled, dataItem) = write.enabled && !CONDITION -> Operate, else ViewOnly - kept honest so the browse never advertises a tier the write path would refuse (MQTT's CanWrite principle), with the caveat above recorded verbatim at the call site. Status-code constants are spelled WITHOUT MqttDriver's "Status" prefix, to match the two this file already declares (BadCommunicationError, BadNotConnected). StatusCodeParityTests strips either spelling and passes. TDD: WriteAsync landed first as a NotImplementedException scaffold - 19 of 20 new tests red on behaviour, 1 (the capability assertion) green on the scaffold - then implemented. MTConnectFactoryTests' "not_writable" characterization test is rewritten to pin the new truth, and is the guard for the concrete-return contract the factory remark explains. Tests: 532 passed / 0 failed (was ~509). Full solution build: 0 errors, 0 warnings; OTOPCUA0001 clean. |
||
|
Joseph Doherty
|
3b8b5a77e1 | fix(mtconnect): share MTConnectError text extraction; pin hollow-error refusal | ||
|
Joseph Doherty
|
9f17d7efa2 | feat(mtconnect): agent-client observation write (HTTP POST, AllowPut surface) | ||
|
Joseph Doherty
|
b5e39e1f32 | feat(mtconnect): parse the write.enabled config block | ||
|
Joseph Doherty
|
e784545844 | docs(plan): MTConnect write-back design + implementation plan; MQTT P3 recorded as already shipped | ||
|
Joseph Doherty
|
de036c62aa | chore(plan): mark the raw-modal picker live-config gate task complete | ||
|
Joseph Doherty
|
8af5742406 |
docs(raw): live /run gate PASSED for the raw-modal picker live config; records Gitea #539
All three legs pass against the patched AbCip fixture: the edit modal and the manual-entry grid both browse the live nine-tag symbol table, and an unreachable device degrades to the graceful error chip while the modal still saves. The gate also found a defect unrelated to this change and filed it as #539: the AbCip discovery path (LibplctagTagEnumerator, LibplctagTemplateReader) passes libplctag the bare host, so an authored non-default EtherNet/IP port is dropped and the browse reaches whatever answers on 44818 — #534's defect surviving in the discovery half. Discovered because the first negative-leg endpoint differed from the fixture only by port, which never reached the wire. |
||
|
Joseph Doherty
|
d88e42b8c4 |
merge: raw modals feed pickers the live merged device config (#468 follow-up closed)
The last dangling #468 item: RawTagModal / RawManualTagEntryModal passed a '{}' stub as GetDriverConfigJson, so the AbCip/TwinCAT/FOCAS pickers' 'Browse controller' captured against no endpoint. Both now resolve the merged Driver+Device config per open and forward a live accessor; review found and fixed a stale-open race (a superseded open could install another device's config). 8 new component-class tests; AdminUI suite 1064/1064. RawBrowseModal's identical race shape deferred to deferment.md §0.1. Live /run gate pending (rig + fixture staged). |
||
|
Joseph Doherty
|
4b88dc6f9a | docs(raw): final-review nits — commit count, closed cross-reference, forced accessibility asymmetry | ||
|
Joseph Doherty
|
c1e8762cd4 |
docs(raw): close the #468 follow-up — raw modals now feed pickers live merged config
Records the outcome in the design doc §6 (including the stale-open race the review found and fixed in both modals), closes the follow-up in the tracking doc and the universal-browser plan, and registers RawBrowseModal's shared race shape in deferment.md §0.1 as the deliberate leftover. |
||
|
Joseph Doherty
|
2c19a50bf9 | fix(raw): a superseded manual-entry open can no longer feed the pickers another device's config | ||
|
Joseph Doherty
|
4e3a6e8497 | fix(raw): a stale open can no longer feed the picker another device's config | ||
|
Joseph Doherty
|
c1c21f3ff8 | test(raw): pin the manual-entry accessor as live, not a captured snapshot | ||
|
Joseph Doherty
|
a2093b4400 | fix(raw): manual tag entry feeds each row's picker the merged device config (#468 follow-up) | ||
|
Joseph Doherty
|
abb9f8a1cb | fix(raw): tag edit modal feeds the picker the merged device config, not '{}' (#468 follow-up) | ||
|
Joseph Doherty
|
e698aee401 | test(raw): FakeRawTreeService for raw-modal component tests | ||
|
Joseph Doherty
|
ebcf73f72c | docs(plan): implementation plan for the raw-modal picker live-config fix | ||
|
Joseph Doherty
|
3b9009da47 |
docs(drivers): tracker catches up with the #468 gate closure; design for the last dangling follow-up
The tracking doc and program-design §9 still showed Wave 0's live gate open —
it closed 2026-08-01 (
|
||
|
Joseph Doherty
|
3c6607d734 |
docs(deferment): reconcile the register with reality — the tracker is empty
The register said "14 issues remain open" and recorded #491 as an open live gate with "no gate-passed commit exists". Both were false: the tracker has been at 0 open since 2026-08-04, and #491's gate passed that morning after finding a production-fatal defect the "code complete" reading had missed. Re-derived every count from its source of truth rather than from this file: the Gitea API (0 open; closure dates for all 14), a parse of every docs/plans/*.tasks.json (31 open tasks across 6 files), and a listing of code-reviews/ (10 unreviewed projects, 3 retained Wonderware modules). That surfaced three things reading the file could not: - §6.2 was missing a live plan file that §5 lists as an open gate, so the two sections disagreed on whether the work existed. - §5 cited that file at a date that does not exist in docs/plans/. - The gate's stated blocker has expired — it needed an operator sign-in, and docker-dev has since disabled AdminUI login, so it is runnable now. Also records a correction rather than a status flip: archreview 03/S4 ("primary gate default-allows on unknown role") is the same shape as a decision deliberately taken in PrimaryGatePolicy.ShouldRecordHistorizedValues, where an unknown role captures. Defensible there, not in a write gate — so closing 03/S4 must not generalise one rule across both. Third round running in which this file's own bookkeeping was what broke, after the dangling pre-amend SHAs and the truncation. The trailer now says so, and says to re-derive rather than re-read. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
4bdce91002 |
docs(historian): live gate for #537 and #538 — both PASSED
Ran on the docker-dev central pair against the local HistorianGateway container talking to the real
AVEVA Historian on wonder-sql-vd03 (VPN up), with the host image rebuilt at
|
||
|
Joseph Doherty
|
e7fbe97759 |
fix(historian): gate historization capture on the Primary, quarantine a poison tag (#537, #538)
Both defects were found by #491's live gate against a real HistorianGateway and filed rather than fixed at the time, because each one's fix is a design decision rather than a patch. #537 — both nodes of a redundant pair captured every value, so the historian received duplicate live samples for every historized tag (measured: the gateway's write counter advanced twice per value and halved when one central was stopped). The recorder now reads a capture gate per value, fed from the new PrimaryGatePolicy.ShouldRecordHistorizedValues through IHistorizationRoleView — a third verdict on the same RedundancyRoleView object, registered unconditionally on driver nodes rather than alongside the alarm section's IRedundancyRoleView (which only exists when that section is enabled; piggybacking is the #511 registration accident). Two things are deliberate and easy to get backwards: * It gates the CAPTURE, not the drain — the opposite of the alarm sink. That queue is in LocalDb and replicates, so the Secondary already holds the Primary's rows. The historization outbox is a node-local FasterLog replicating to nobody, so a drain-only gate would leave the standby appending forever to a queue no one empties: unbounded disk, then a flood of stale duplicates the moment it is promoted. Entries a node already captured are still drained by that node. * The peer it stands down for must RECORD THE SAME REFS, not merely hold the same rows — a Primary in this node's own per-cluster mesh. A node carrying no cluster-scoped role cannot establish that and never stands down: under a fleet-wide election the elected Primary is usually running a different cluster's configuration, and deferring to it would end this cluster's historization silently, with the drain reporting success on an empty outbox forever. That is the scope mismatch that once suspended the alarm drain on every node of the rig. An unknown role captures, matching the alarm drain — a duplicate sample is recoverable, a hole in the trend is not. #538 — the drain writes every per-tag group and then acks all-or-nothing (deliberate: RemoveAsync truncates THROUGH an id, so partial acking could drop older unwritten entries). One permanently failing tag therefore pinned the FIFO head while every healthy tag in the batch was re-submitted to the historian on every pass, indefinitely — duplicate samples and an outbox that never truncated, observed live at 8.3 MB from a single unprovisionable tag. The drain now tracks consecutive failures per historian tag and abandons that tag's entries once it exceeds MaxWriteAttempts (new, default 5), so the head advances. The counter only advances on a pass where some OTHER tag wrote successfully: that is what separates "this tag is poison" from "the backend is down". A count-only quarantine looks identical on the poison-tag case and silently discards the whole backlog during an ordinary gateway restart, turning a duplicate-samples bug into a lost-history one. Abandonment is also deferred until every failing tag is over budget, so nothing is discarded on a pass that could not have advanced the head anyway. Each abandonment logs a Warning naming the tag; the total rides RecorderStatus.AbandonedEntries. Capacity also gains a non-zero default (500,000 entries) as the disk backstop for the one case the quarantine deliberately cannot resolve — a historian refusing every write, where retaining is correct. Set 0 to restore unbounded retention. Every new test was falsified by reverting the production change it guards, and each one reddens alone: * drop the backend-reachable evidence -> only A_backend_that_fails_every_tag_abandons_nothing * never abandon -> only A_permanently_failing_tag_is_abandoned_so_healthy_tags_stop_being_rewritten * latch the capture gate at construction -> only The_capture_gate_is_re_read_per_value... * treat any Primary as a scope peer -> only the two scope-mismatch cases Full suite green. (Four OpcUaClient.Browser failures during the run were the opc-plc fixture being down on the shared Docker host; brought it back up and they pass.) Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
e77ea4aa73 |
docs(historian): close #491 — read-back and EnsureTags proven against wonder-sql-vd03
The 2026-08-01 run closed every leg except read-back, which no reachable historian could
serve that day (the LAN instance accepts writes but serves no reads under a licence tag-count
violation; the licensed one needed the VPN). With the VPN up the run was repeated against
wonder-sql-vd03 and the remaining legs pass:
- EnsureTags provisioning: ensured=2, failed=0 — the path the LAN box could not exercise at
all (it is RemoteGrpc-only and that CAP was not listening).
- Read-back: two Modbus registers carrying DISTINCT NON-ZERO constants were historized, and
each read back as its own value (HR20 -> 8224, HR3 -> 120, quality 192). A first attempt
read back 0, which proves nothing — the same trap #536 recorded, so the proof was rebuilt
on values a crossed or fabricated answer could not fake.
Found during the run and FILED rather than fixed:
- #538 — one permanently-failing tag re-writes every healthy tag on every drain pass, forever.
DrainOnceAsync writes EVERY per-tag group and then acks all-or-nothing, so a tag whose
historian tag does not exist stops the FIFO head advancing while the healthy tags' values
are re-submitted every drain interval; the outbox never truncates (Capacity defaults to 0 =
unbounded, observed at 8.3 MB). The all-or-nothing ack is deliberate — RemoveAsync truncates
THROUGH an id, so partial acking could drop older unwritten entries — so the fix changes the
outbox contract and does not belong in this commit.
Also re-confirmed live: EnsureTags provisioning fires only for newly-ADDED historized tags, so
flipping an existing tag's historianTagname provisions nothing (Issue 5 of
2026-06-27-historian-gateway-integration-issues.md).
No production code changes — the recorder fix shipped in
|
||
|
Joseph Doherty
|
ad74473452 |
fix(historian): the recorder died on its first value — live gate for continuous historization (#491)
Ran #491's live gate against a real ZB.MOM.WW.HistorianGateway talking to a real AVEVA Historian, and it found a production-fatal defect on the first value it ever saw. ContinuousHistorizationRecorder.OnValueChangedAsync touched `Self` after awaiting IHistorizationOutbox.AppendAsync(...).ConfigureAwait(false). ConfigureAwait(false) discards Akka's ActorTaskScheduler synchronization context, so `Self` — which reads ActorBase.Context — threw NotSupportedException. The throw escaped into supervision, which RESTARTED the actor; because production seeds an EMPTY ref set (the deployed set arrives later via UpdateHistorizedRefs), the restart silently discarded every historized ref. Net effect: one value captured, then nothing, with no further error to show for it. Fixed by capturing `IActorRef self = Self` before the await — the idiom the GetStatus handler already applied to Sender. Thirteen unit tests missed it because InMemoryOutbox.AppendAsync returned ValueTask.CompletedTask: a completed ValueTask never yields, so the continuation ran inline with the context still installed. The production FasterLog outbox fsyncs per entry and does yield. The first regression test was itself vacuous — `await Task.Yield()` posts back to the CAPTURED context, so the append still completed on the actor thread and reverting the fix left the test green. The fake must complete on a POOL thread (Task.Delay(1).ConfigureAwait(false)), as a real fsync does; with that, reverting the fix reddens exactly one test. The rig could not run this gate at all: docker-dev carried ServerHistorian__* but no ContinuousHistorization__* keys, so the recorder could never spawn. Both centrals now carry them (default OFF — the recorder needs ContinuousHistorization:Enabled AND ServerHistorian:Enabled), with the outbox on the per-node named volume so it survives a container recreate. ServerHistorian__UseTls was hard-coded true and is now parameterized; a plaintext h2c gateway was unreachable without it. Gate results: outbox append, drain, WriteLiveValues accepted by a real historian, and restart convergence (capture resumes after a restart with no new deployment) all pass. The read-back hop is NOT proven — the LAN historian accepts writes but serves no reads under a "License violation: Database tag count exceeds license tag count", and the licensed corporate one needs the VPN. EnsureTags provisioning is likewise unexercised there (RemoteGrpc-only, and that CAP is not listening). #491 stays open for the read-back hop alone. Also recorded, not fixed: the recorder has NO Primary gate, unlike the alarm store-and-forward sink, so both nodes of a redundant pair send duplicate live values. Gating it would introduce a failover window with no capture — that is a design decision, not a cleanup. Evidence: docs/plans/2026-08-01-continuous-historization-live-gate.md Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
6b3da93151 |
fix(browse): commit the browsed data type, and fix the probe classification the fixture was hiding (#536)
A browse-committed AB CIP / TwinCAT / FOCAS tag bound with its model's DEFAULT
data type, not the one the device reported. A browsed REAL committed as
`{"tagPath":"Pump_1_Speed","dataType":"DInt"}` — and those three drivers decode
the wire bytes by that field, not by the raw row's OPC UA DataType — so the tag
read four bytes of a float through an integer decoder and published a plausible
wrong number with no error raised anywhere. FOCAS was missed when #536 was filed.
The type is CARRIED, not derived. Reverse-mapping the leaf's driver-agnostic
DriverDataType looks like the smaller fix and is wrong: SInt/Int/DInt/USInt/UInt
all collapse into Int32 on the way out of the driver, so that version would bind
a 16-bit INT as a 32-bit DINT while appearing to fix the REAL. Drivers now state
their own type name on DriverAttributeInfo.NativeDataType, which reaches the
picker as AttributeInfo.AddressFields["dataType"] and the commit mapper from
there. A leaf that states nothing keeps the model default — the pre-#536
behaviour, kept deliberately as the fallback rather than a guess.
Numeric strings are refused before parsing: Enum.TryParse accepts "9" for any
enum and Enum.IsDefined then confirms it, so an unguarded parse would bind a tag
to a type nobody named. A test caught this in the first version of the guard.
--- and the other half, which the fixture was hiding ---
Test Connect on a device with NO authored tags — every AB CIP device at the
moment an operator first presses the button — reported "CIP handshake failed"
against a healthy controller. That was written down as a fixture limitation
("ab_server doesn't implement @raw_cpu_type"). Both halves of that were wrong:
* @raw_cpu_type is not a libplctag special tag at all (only @raw, @tags and
@udt/ are), so it goes out as an ordinary symbolic name that no controller
defines, and the controller's "tag does not exist" IS the successful answer.
* ab_server answered an unresolvable tag path with CIP 0x03 where a Logix
controller answers 0x04 — ErrorBadParam vs ErrorNotFound, which the probe
classifies oppositely. Fixed in vendored patch 0002.
* With the fixture then telling the truth, the real defect surfaced:
AbCipDriverProbe.IsReachableException matched the native C status spellings
("STATUS_NOT_FOUND"), but libplctag.NET throws with the MANAGED enum name
("ErrorNotFound"). Those substrings can never match, so the reachable
classification could not fire on the exception path — which is the path every
no-authored-tags probe takes. This was failing on real hardware too, not just
against the sim. It now recovers the Status and defers to IsReachableStatus,
so there is one classification rather than two that can drift.
Rejected: switching the fallback to @tags. libplctag builds a tag-listing tag
with NO network I/O, so it reports OK against any TCP listener — measured green
against a Mosquitto broker.
Verification: full solution suite green. Falsification runs confirm exactly the
intended tests go red — 9 for the type carry, 3 (the reachable statuses only) for
the probe classification. Live on docker-dev: browse-committed TestString/
Motor_Run/TestSINT now store String/Bool/SInt, alongside last session's
Pump_1_Speed still holding the wrong DInt. TestSINT is the falsifier — its row
DataType is Int32, identical to TestDINT's, so no reverse map could have produced
"SInt".
Known consequence: pre-existing browse-committed tags keep their wrong dataType;
the fix applies to new commits only.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
|
||
|
Joseph Doherty
|
c2c3c52274 |
fix(abcip): unblock the controller-browse gate, and fix the decoder it exposed (#468)
The AbCip controller browse could never be verified end to end: libplctag's ab_server answers CIP service 0x55 (Get Instance Attribute List) on the Symbol Object class with "unsupported", so the `@tags` walk had no backend. #468's own definition of done offered a sim that serves `@tags` as an alternative to real hardware; this is that. The fixture now applies a vendored patch teaching ab_server to serve the Symbol Object listing, with partial-transfer paging. Correctness is checked against libplctag's own `list_tags_logix` rather than against our decoder — our decoder is the thing under test, so its agreement would prove nothing. The clone is also pinned to a concrete tag: the Dockerfile called `--branch release` "pinned" while tracking a branch that moves. That immediately exposed a real defect. CipSymbolObjectDecoder advanced to an even byte boundary after each symbol name, but entries are packed with no padding — libplctag walks them by sizeof(entry) + string_len, and a captured reply confirms it. Any odd-length name therefore desynchronised every entry behind it, and odd-length names are ordinary. In the live browse below, `Motor_Run` is 9 bytes and first on the wire, so the old decoder would have shown that one tag and silently dropped the other eight. Every existing multi-entry test passed throughout, because the test builder padded exactly as the decoder did: the two agreed with each other and neither agreed with a controller. The builder no longer pads, and the layout is pinned by a captured reply (CipSymbolObjectWireCaptureTests) instead of by a second statement of the assumption. Restoring the alignment step reddens exactly the three new tests and none of the 29 others. Live gate on docker-dev, all three of #468's criteria: - populated tree — all nine seeded tags render as leaves, in wire order - refresh re-captures - leaf commit — Pump_1_Speed + TestDINT saved with their CIP tag paths The gate also surfaced two things it was not asked about: browse-commit drops the discovered data type, so a browsed REAL commits as a DINT and the driver decodes it as one (filed as #536), and Test Connect fails against this fixture regardless of the device because it probes `@raw_cpu_type` (documented). TwinCAT and FOCAS still have no browsable backend; #468's AbCip half is closed, theirs is not. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
1a1dbc3f00 |
fix(uns): enforce the UNS segment rule on every authoring path (#535)
An area or line could be named "Packaging" or "Line 1", save cleanly, and then fail the NEXT deployment with UnsSegmentInvalid. Because a deployment snapshots the whole database, one badly-named area blocked EVERY cluster's deploy - and whoever hit that was usually not whoever typed the name. Only the equipment form carried the rule; the deploy gate held its own private copy. The rule now lives once, on Configuration.Validation.UnsSegment, and both ends call it. DraftValidator no longer restates it. Enforced in UnsTreeService, not just on the forms: - Create/Update Area, Line and Equipment reject an invalid name before touching the row. - The CSV import reports a bad name as an ERROR, not a skip - a skip means "you already have this row", which would tell the operator the equipment exists when it does not. Good rows in the same batch still insert. A form attribute could never have covered the import, and with no bUnit in this repo a rule only a form enforces is also a rule no test can reach. The forms keep an attribute for the immediate message, but it now references the shared pattern rather than restating it - a second copy is how these drifted apart. One behaviour change beyond the fix: UnsSegment.AttributePattern admits "_default", which the equipment form's hand-written literal rejected. The form was stricter than the gate it was mirroring, so a _default row was uneditable through the AdminUI. Tests: 41, falsified with three production mutations - dropping the CreateArea guard reds exactly the CreateArea rejections, turning the import error back into a skip reds only the import test, and narrowing AttributePattern reds only the attribute/IsValid agreement. Every rejection is paired with a name that must still be accepted, or a guard that refused everything would pass. Live-verified on docker-dev: the area modal now refuses "Packaging" in place with the rule spelled out, "packaging-b" saves, and a CSV import of two rows reports Inserted 1 / Skipped 0 / Errors 1 naming the offending row - the path no form attribute reaches. Rig cleaned up and a further deployment sealed. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
9303289d05 |
docs(plan): correct the self-form watchdog record — rejected, TCP guard was interim
Family-wide docs correction (paired with scadaproj 6d222ea4). This plan's superseded
banner was right but its task body still read as if the watchdog were merely patched.
- Dated 2026-08-01 correction section (history preserved): the timer calling
Cluster.Join(SelfAddress) was implemented and REJECTED in both repos after live
testing — outside Akka's join handshake, it fires mid-join on a routine standby
restart (peer alive, join stalled behind stale-incarnation removal) and forms a
permanent second cluster at the same address. ScadaBridge measured a split unhealed
after 90s; Task 8 hit the same shape live here.
- The 'a booting node only self-forms when NO seed answers InitJoin' claim in the Task 2
doc comment is marked FALSE for the watchdog: that guarantee belongs to Akka's native
FirstSeedNodeProcess, which runs inside the handshake; the timer does not, and fires
while a seed is answering.
- Task 8's TCP reachability guard is recorded as an INTERIM patch, not the fix — the
race survived it, which is why the watchdog was deleted rather than patched.
- Shipped design restated: self-first seed ordering (AkkaClusterOptionsValidator +
SelfFirstSeedBootstrapTests) plus the opt-in, default-OFF bootstrap guard (
|
||
|
Joseph Doherty
|
a88cefd5ee |
docs(security): close the stage-7 gate's UNS gap — legs 12-15
The first pass could not exercise UNS-realm scope selection because SITE-A had no UNS. Authored one on docker-dev (area -> line -> equipment) and re-ran that part: the picker's UNS column offers UnsArea/UnsLine/Equipment, selecting the equipment resolves, and the grant deploys Sealed. Leg 15 is the one worth keeping. Re-pointing the SITE-A grant at MAIN's equipment is the only rejection whose id is REAL, so a check that asked only "does a row with this id exist?" would accept it and confer authority across a cluster boundary. The list page and the deploy gate both catch it and both name the owning cluster. Also found and filed (Gitea #535, not fixed here): the /uns area and line modals do not validate the UNS segment format, so "Packaging" and "Line 1" saved happily and then failed the deploy with UnsSegmentInvalid. Only the equipment form carries the rule. A deployment snapshots the whole database, so one bad area name blocks every cluster's deploy - the same authoring-vs-gate gap stage 7 just closed for NodeAcl rows. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
3796c0bf22 |
merge: ACL scope picker + authoring-time resolution (#520 stage 7)
Closes #520 — all eight stages done and live-gated. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
d542ab5b7c |
feat(security): author ACL grants from the real tree, not free text (#520 stage 7)
The last stage of #520. Enforcement landed in stages 5-6; this closes the authoring end, where a grant could be saved, deployed and sealed green while resolving to nothing. Scope kind and scope target are now chosen TOGETHER from a picker over the cluster's raw and UNS trees (AclScopePickerModal -> AclScopeTree, backed by IAclScopeService). They were previously an unrelated dropdown and a free-text box, so "RawDevice + a TagId" was a well-formed row. Because the trees are rooted at the grant's own cluster and each node supplies its own kind, NodeAclScopeCrossCluster, NodeAclScopeIdUnexpected, NodeAclScopeIdMissing and NodeAclScopeKindRetired all become unauthorable by construction. CheckScopeAsync mirrors DraftValidator.ValidateNodeAclScopes rule for rule and blocks the save when a scope resolves to nothing - re-checked on submit, not trusted from form load, because a row whose entity was deleted since is exactly where the operator never touches the picker. The list page marks such rows unresolved with the reason. The principal field suggests known principals and reports how many live keys reach the one typed; reaching nobody warns rather than blocks, since authoring ahead of minting is legitimate. Deliberate departures from the plan line, all recorded in the design doc: - The banners are reshaped, not removed. "Not enforced" stopped being true at stage 6, but enforcement is still a per-node flag the AdminUI cannot see, so a narrow grant really does confer nothing on a default-configured node. - A Cluster row produces a NULL ScopeId. The deploy gate rejects one that carries an id, so filling in the cluster id would author undeployable rows. - AclScopeTree is a new component rather than a third mode on RawTree, which already carries the /raw editing tree and the equipment reference picker. - UNS tags are not offered: a UNS signal is a UnsTagReference, and a Tag grant already covers the raw node and every UNS projection of it. Also fixed: the ACL editor had a DataAnnotationsValidator and no ValidationSummary, so an invalid NodeAclId made Create a silent no-op. Tests: 30 in AdminUI.Tests/Acls, falsified with six production mutations - each turned exactly the intended tests red and no others. Live gate PASSED on docker-dev, 11 legs; both central containers asserted on the same image by container id. The pair that matters is legs 8 and 11: the page and the deploy gate reject the same row with the same reason. Legs 5 and 9 are the positive controls - without them the gate passes against a page that refuses everything. Not covered: UNS-realm scope selection (SITE-A has no UNS areas). Rig restored to zero ACL rows. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
d4389f57a2 |
merge: node ACL enforcement on read, browse and subscribe (#520 stage 6)
Completes data-plane enforcement behind Security:Acl:Enabled (still default false). Read — every attribute, not just Value — Browse, TranslateBrowsePath, CreateMonitoredItems, SubscribeToEvents and SetMonitoringMode now consult scoped NodeAcl grants. With the flag off nothing changes. LIVE-GATED on docker-dev: site-a-1 with the flag on, central-1 on the same artifact as the control. Revoking a grant stopped a running subscription mid-stream — client told (0x801F0000), 25 seconds of silence, feed resumed by itself on restore. Full transcript in the design doc. Four decisions worth knowing: - Browse needed ancestor visibility, or every grant narrower than the cluster is authored, shipped and unreachable — a client reaches a deep node by browsing the nodes above it, and the trie inherits downward only. It is a separate non-inheriting marker, NOT a synthesized Browse grant on each ancestor: the additive walk would carry such a grant onto every sibling subtree. - Revocation had to reach LIVE subscriptions. Every other operation re-asks per request; a subscription is authorized once and streams, and streaming is how machine consumers read this server. A denied item is queued BadUserAccessDenied and then DETACHED from its fan-out, not set MonitoringMode.Disabled — disabling discards the queued notification, so the client would be silenced without being told why. Detaching is also reversible. - The decision cache holds the grant WALK, not the decision. Grants are fixed for a generation; a key's expiry moves with the clock, so caching the whole decision would let an expired key work until the next deploy. - TranslateBrowsePath is gated too, or a client refused a browse just asks TranslateBrowsePathsToNodeIds for the same path. Three defects found and fixed en route, all the same shape — an `object`-typed SDK parameter that the wrong cast reads as null, disabling a gate silently: ContinuationPoint.NodeToBrowse is a NodeHandle (browse source gate), and SubscribeToEvents' sourceId is too (event gate, which would have let an ungranted session receive every alarm on an equipment whose values it was refused). Also: UserAuthorizationState's staleness ceiling couples the authorizer's clock to the evaluator's, and any divergence denies EVERYTHING with a verdict indistinguishable from "no grant matched". Consumer-visible: with the flag ON, a read-only consumer now needs a Read grant. The umbrella index's previous claim that reads and subscriptions were ungated in either mode no longer holds. Full solution suite green. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
c4f537a60c |
docs(security): record the stage 6 live gate — PASSED on docker-dev
site-a-1 (driver-only, no ConfigDb) with Security:Acl:Enabled=true; central-1
on the same artifact as the A/B control. 11 legs.
The two that only a live rig could settle, as a transcript:
13:49:39 SiteAOnly = 53 (0x00000000) streaming ~1/s
13:49:40 SiteAOnly = (0x801F0000) revoking deploy lands; client TOLD
… 25 seconds of silence, ~25 updates that never arrived …
13:50:05 SiteAOnly = 157 (0x00000000) restoring deploy; feed resumes
53 -> 157 across the gap: the device counter ran throughout, so delivery
stopped, not the source. The 0x801F0000 frame is exactly what the
MonitoringMode.Disabled implementation would have swallowed.
Also recorded: the leg that distinguishes working enforcement from a trie that
denies everything is resolvedScopes=1 in the install log, not any denial.
Stated plainly as NOT covered: a UNS-realm grant (the rig's SITE-A has no UNS
equipment) and alarm-command scoping (no alarms there). Both covered offline,
neither seen live.
Two rig facts worth keeping: :9200 round-robins central-1/central-2, and a
stale central-2 rejected the deploy outright because its image predated the
stage-3 scope kinds — image parity must be asserted per container, not assumed
from one build. And SITEA-modbus had an empty DriverConfig, so it ran as a stub
serving nothing; until that was fixed every read returned
BadWaitingForInitialData and no allow leg could be told from a denial.
Rig left as found.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
|
||
|
Joseph Doherty
|
2aa42f90eb |
fix(security): the ACL startup warning described stage 5's scope, not stage 6's
It told the operator "Read, Browse and Subscribe remain ungated" — which stage 6 is precisely the change to. A warning that misstates what a security control does is worse than no warning, because it is read as authoritative. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
8cdbcb9823 |
fix(security): resolve the event-subscribe gate's source through IsHandleInNamespace
SubscribeToEvents' sourceId is the manager HANDLE, not a NodeState — the same object-typed trap as ContinuationPoint.NodeToBrowse, found the same way. The `sourceId as NodeState` cast compiled, always yielded null, and silently disabled the gate: an ungranted session could subscribe to an equipment's event notifier and receive every alarm it raises, including the condition fields, while every value on it was refused. Now uses the SDK's own IsHandleInNamespace, which cannot be wrong about the type. The new test passes a null monitored item deliberately: the gate must deny without touching the item, which also makes the test a discriminator rather than a restatement — against the broken cast the call falls through to the base implementation, which does dereference it, so the test fails loudly. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
ce404df970 |
fix(security): count only this page's references in the browse-filter summary
On a BrowseNext the references list already carries the earlier pages, so the summary's denominator counted them again. Log accuracy only — the filtering itself was already scoped to this call's appended range. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
00aa486677 |
feat(security): enforce node ACLs on read, browse and subscribe (#520 stage 6)
Completes data-plane enforcement behind Security:Acl:Enabled. Read (every attribute, not just Value), Browse, TranslateBrowsePath, CreateMonitoredItems, SubscribeToEvents and SetMonitoringMode now consult scoped NodeAcl grants. With the flag off nothing changes. Four things went beyond the stage as written, each because leaving it out would have made the feature look like it worked: - Browse needed ancestor visibility. The trie inherits downward, and a client reaches a deep node by browsing the nodes above it, so without this every grant narrower than the cluster was authored, shipped and unreachable. It is a separate non-inheriting marker, NOT a synthesized Browse grant on each ancestor: the additive downward walk would carry such a grant from Plant onto Plant/Siemens/dev9 and every other sibling subtree. Pinned by a test that goes through the evaluator — probing the trie helper directly passes against the broken design, because a sibling is absent from the trie either way. - Revocation had to reach LIVE subscriptions. Every other operation re-asks per request; a subscription is authorized once and then streams, and streaming is how machine consumers actually read this server — so stopping only the re-asking operations would have made revocation close to cosmetic. IDataPlaneAuthoritySource gained a Changed event. A denied item is queued BadUserAccessDenied and then DETACHED from its MonitoredNode2 fan-out, not set MonitoringMode.Disabled. Disabling discards the item's queued notifications, so the client is silenced without ever being told why. The over-the-wire test caught this; the unit-level path reports the item as stood down either way. Detaching is also reversible, so a restored grant resumes the feed with no client action. - The decision cache holds the grant WALK, not the decision. Grants and the trie are fixed for a generation; a key's ExpiresUtc passes with the clock, so caching the whole decision would let an expired key keep working until the next deploy — the stage-5 open-session hole in a subtler form. - TranslateBrowsePath is gated, without which browse filtering is decorative: a client refused a browse asks TranslateBrowsePathsToNodeIds for the same path. Found en route: UserAuthorizationState's staleness ceiling couples two clocks. MembershipResolvedUtc is stamped from DataPlaneAuthorizer's TimeProvider while IsStale is evaluated against TriePermissionEvaluator's; they are the same instance in production and diverge the moment anything injects a test clock, at which point the evaluator denies EVERYTHING with a verdict indistinguishable from "no grant matched". Stage 5's claim that these predicates cannot fire was accidentally true; they are now disabled explicitly. Also fixed: ContinuationPoint.NodeToBrowse is the manager HANDLE, not a NodeState. The declared type is object, so the wrong cast compiled cleanly and silently yielded null — skipping the browse source gate entirely. Falsified: disabling the transit marking reddens exactly the three tests that assert transit works; implementing transit as an ancestor grant reddens the sibling-leak test; skipping the fan-out detach reddens the revocation test. Full solution suite green. The stage 8 live gate has NOT run yet. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
1ccc74005a |
merge: node ACL enforcement on write, alarms and HistoryRead (#520 stage 5)
Behind Security:Acl:Enabled (default false), scoped NodeAcl grants now decide writes, alarm commands and HistoryRead. HistoryRead had no authorization check of any kind before this. Read, Browse and Subscribe stay ungated in both modes (stage 6). The ACL decision REPLACES the role check rather than chaining with it. Chained, a grant would have to confer the server-wide role as well as pass the scope check, and only Cluster-scoped grants confer a role — so every narrow grant would stay inert and the flag would appear to do nothing. The consequence for tests: a deny-only test is vacuous here because the role gate denies too, so the load-bearing test is an allow — a roleless session writing on its grant alone. Fixes a live defect found on the way in: the ACL principal never reached the session. RoleCarryingUserIdentity derived DisplayName from the presented token, which is the static username "apikey" for every key-bearing client, so the trie's matching key was dropped at the identity boundary and every rejected write audited as "apikey". Beyond the stage as written: - revocation now reaches an already-open session (both a revoked key and a removed grant), rather than persisting until the client chooses to reconnect - UserAuthorizationState is built per call, not per session; its freshness predicates are LDAP-shaped and cannot fire under API keys, so they are documented as vestigial rather than left looking load-bearing - the trie is built inside DataPlaneAuthorityState.Apply, not as a second wiring — an uninstalled trie denies everything, indistinguishable from working enforcement - both IApiKeyStore implementations widened from Cluster-scoped grants to the full grant set; ApiKeyInterimRoleMapper still narrows, pinned by tests The stage 8 live gate has NOT run — all verification is offline. Every failure path denies, so a wiring mistake presents as a total outage of the three gated operations rather than a subtle permission bug. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
f692f6afe4 |
feat(security): enforce node ACLs on write, alarms and HistoryRead (#520 stage 5)
`Security:Acl:Enabled` (default false) makes scoped `NodeAcl` grants decide data-plane authorization at the three seams that have one, and gives HistoryRead a check it never had. Read, Browse and Subscribe stay ungated — stage 6. The ACL decision REPLACES the server-wide role check rather than chaining with it. Chained, a grant would have to confer the role as well as pass the scope check, and only Cluster-scoped grants confer a role — so every narrow grant would have stayed inert and the flag would have looked inert too. The load-bearing test is an ALLOW: a session carrying no roles at all writing successfully on the strength of its grant. A deny-only test passes against a build where the ACL is never consulted, because the role gate denies too. Found en route: the principal never reached the session. The authenticator passed it as OpcUaUserAuthResult.DisplayName, but RoleCarryingUserIdentity derives DisplayName from the presented token — for a key-bearing client that is the static username `apikey`, identical fleet-wide. So the trie's matching key was dropped at the identity boundary, and the audit ClientUserId on a rejected write recorded `apikey` for every consuming system. Principal and ApiKeyId are now carried explicitly. Beyond the stage as sketched: - Revocation reaches an OPEN session. Every decision re-checks the authenticating key against the current snapshot; without it a key revoked by a deployment kept its authority until the client chose to reconnect, which for a machine-to-machine consumer can be indefinitely. - UserAuthorizationState is built per CALL, not per session. The per-session shape exists so an LDAP session can re-resolve membership that changed out of band; nothing can change out of band under API keys, so the generation stamp is the whole invalidation signal and a per-call record picks it up for free. IsStale/NeedsRefresh therefore cannot fire here and are documented as vestigial. - The trie is built inside DataPlaneAuthorityState.Apply, under the same generation as the snapshot, rather than by a second wiring at the publish actor. An uninstalled trie denies everything — indistinguishable from working enforcement. Deleting the install turns 13 of the 22 new tests red. - The deploy log reports what the BUILD dropped instead of re-deriving it. The old count agreed with the builder on one of its four drop reasons; a rule with a blank ScopeId, a retired scope kind, or a path not starting at a realm vanished with the log reading zero. Both IApiKeyStore implementations widened to the full grant set. The narrowing was redundant — ApiKeyInterimRoleMapper applies the same filter itself — and became a trap once scope was enforced. Pinned by asserting the roles are unchanged, with the two grants carrying different flags so it cannot pass by accident. Falsified by mutation: enforcement never engaging, the write seam ignoring the ACL, the realm dropped from the scope, the revocation check removed, and the trie never installed each turn the intended tests red and no others. Full solution suite green. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
9499099028 |
merge: ACL scope model + role/identity fixes + four real test failures (#520 s3, #532, #533, #534)
Five commits, four Gitea issues closed. #520 stage 3 — NodeScope becomes (ClusterId, Kind, Segments) with a variable-length path, and AclScopePathResolver finally provides the scopePaths producer the trie builder's required argument had never had. Closes two of the four documented wire-up blockers: raw-realm nodes are now representable, and an unresolvable scope is dropped with a diagnostic instead of planted where the walk cannot reach it. Node ACLs remain unenforced — that is stages 5/6. #532 — Cluster:Roles had no OTOPCUA_ROLES fallback despite two doc sites promising one, so a node configured only through the env var joined the cluster with no Akka roles: invisible to every role query, un-deployable, and silent about it. The field fix on wonder-app-vd03 was a registry edit. Now applied once, in AddOtOpcUaCluster, plus the four open-coded ClusterId derivations collapsed into RoleParser. #533 — three string-identity rules in one authorization path, working only because both sides held the same NodeAcl instances. One rule now: identifiers compare OrdinalIgnoreCase, path segments compare Ordinal. #534 — both libplctag drivers carried Port on their create-params and consumed it nowhere, so libplctag substituted 44818 and the connection went to whatever answers there. A production defect: on a plant network that is another PLC, read as Good. Also fixed: three integration failures that had stopped catching what they guard — a harness that stopped spawning the redundancy singleton after the Phase 6 re-home (48s timeout, both nodes "role unknown"), a hand-written driver-type list that could not see a driver registered without a probe, and an AbCip probe config still in the retired v2 shape, exercising the fallback its own comment says it avoids. The AbLegacy PCCC fixture was deployed for the first time, one port per family. Full solution suite green, zero failures — AdminUI.Tests 969, Host.IntegrationTests 211/4 skipped, AbLegacy integration 4/4 (never passed before). Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
01538464b7 |
fix(abcip,ablegacy): honour the authored EtherNet/IP port (#534)
Both libplctag drivers carried Port on their tag-create params and consumed it nowhere. The runtimes set Tag.Gateway to the bare host, and libplctag does not fail on a portless gateway — it substitutes 44818. So an authored port was not ignored in the harmless sense; the connection was silently REDIRECTED to whatever answers on the default port. On a plant network that is another PLC, and the driver would read its tags and report them Good. This is the root cause of #534's four AbLegacy integration failures, and the explanation for every observation in that issue: with each PCCC family moved to its own port (44819/44820/44821), the driver still dialled 44818 and kept talking to the AB CIP fixture's ControlLogix container. Hence PCCC-against-ControlLogix errors under every server-path / client-cip-path permutation — the permutations were never reaching a PCCC server at all. Established by a direct libplctag 1.5.2 probe outside the repo, which answered #534's open question: PCCC works fine against ab_server for all three families, with path="1,0" AND with path=null (only path="" throws ErrorBadData). Since the library could reach the simulators and the driver could not, the difference had to be in our Tag construction, and Gateway was the only field that differed. Same probe confirmed libplctag accepts host:port in the gateway attribute, which is what the fix emits. AbCip has the identical defect and is fixed alongside. It has never been observable there: every AbCip fixture profile binds 44818, so no test that driver has could distinguish "uses the authored port" from "always uses 44818". A fixture that only ever exercises the default value cannot falsify a rule about non-default values, however many tests point at it. The port is now emitted unconditionally rather than only when non-default, so the value does not depend on libplctag's own default; a port <= 0 still falls back to the bare host rather than producing "host:0". The load-bearing guard is the AbLegacy integration suite, which addresses each family on a non-default port and cannot pass unless the port is honoured — 4/4 green, having never once passed before. Unit tests in both drivers pin the composition for machines with no fixtures. Falsified both ways: reverting the runtime line reddens all 4 integration tests; dropping the port > 0 guard reddens exactly the two fallback cases. AbCip integration 10/13 (3 pre-existing skips), AbCip unit 345, AbLegacy unit 219. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
b13bdc154a |
test(ablegacy): deploy the PCCC fixture, one port per family (#534)
The AbLegacy integration suite has never run against a simulator. /opt/otopcua-ablegacy/ did not exist on the Docker host — the stack was written but never deployed — and every ab_server across BOTH ab fixtures bound :44818. So the AB CIP fixture's ControlLogix container (up for two weeks) answered this suite's TCP reachability probe, the suite declined to skip, and it ran SLC500-mode PCCC against a ControlLogix. Four failures that read as "fixture missing". Two structural problems, both fixed here: - The theory parameterises over all three PCCC families, but only one container could ever exist on the shared port, so at least two cases failed on every conceivable run. The suite was un-greenable by construction, and AB_LEGACY_COMPOSE_PROFILE existed to paper over it by filtering the theory down to whichever container happened to be up. - A reachability probe that cannot tell one simulator from another is not a skip gate. It answered "is some ab_server listening", which is a different question from the one the caller is asking, and reported the difference as a test failure. Each family now has its own host port — 44819 slc500, 44820 micrologix, 44821 plc5 — so all three run together and alongside AbCip on 44818 (ab_server still listens on 44818 inside each container; only the host mapping differs). Skip is per-family via SkipReasonFor(profile); the class-level gate runs if ANY family is up so one down container cannot silently skip the other two. AB_LEGACY_COMPOSE_PROFILE now applies only in AB_LEGACY_ENDPOINT-pinned real-hardware mode, where the family genuinely is a property of the one device. This finally makes true what the fixture's XML doc has claimed from the start — that it and the AB CIP fixture can point at different containers simultaneously. Deployed to /opt/otopcua-ablegacy/ on 10.100.0.35; all three containers running. The 4 tests still FAIL, and are left red on purpose — see #534. Now aimed at correct-family simulators, the PCCC exchange itself returns BadCommunicationError for every server-path / client-cip-path permutation, reproduced outside the test harness via the driver CLI. That is a true signal about the driver (or about libplctag's PCCC support against ab_server) that skipping would hide. What changed here is that the failure is now diagnostic instead of an artifact of a port collision. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
df340b8bba |
fix(tests): three real failures the integration suite had stopped catching
None of these were fixture absence. Each is a guard that went quietly wrong and then reported the wrong thing. PrimaryGateFailoverTests — per-cluster mesh Phase 6 ( |
||
|
Joseph Doherty
|
06c56d7719 |
fix(cluster,acl): OTOPCUA_ROLES fallback + one string-identity rule (#532, #533)
Two independent defects surfaced in review of #520 stage 3.
#532 — Cluster:Roles had no OTOPCUA_ROLES fallback, despite two doc sites
promising one. A node configured only through the env var — which drives ALL of
Program.cs's conditional wiring, so it reads as the authoritative knob — bound an
empty Roles array and joined the cluster carrying no Akka roles at all. It ran,
served, and logged nothing wrong, while central reported "no driver-role nodes
are Up" and every deployment to it timed out. Not hypothetical: the field fix on
wonder-app-vd03 was to hand-add Cluster__Roles__0/1 to a Windows service's
registry environment. Anyone reading the code to diagnose that box would have
concluded from the XML doc that the fallback was already applied.
AddOtOpcUaCluster now post-configures AkkaClusterOptions through the shared
RoleParser.ResolveEffectiveRoles, so BuildClusterOptions (Akka's own member
roles), ClusterRoleInfo, the ControlPlane singleton scope and
ConfigDbApiKeyStore's ClusterId all see one answer. Two sibling validators
(ConfigSource, Telemetry) read Cluster:Roles directly and were exempting the
exact node they exist for — an env-only driver-only node has no ConfigDb and
could sit on ConfigSource:Mode=Direct unchallenged. They resolve the same way now.
Also collapsed the four open-coded `Roles.FirstOrDefault(IsClusterRole)` +
ClusterIdFromRole copies into RoleParser.ClusterIdFromRoles. They had drifted:
three classified the cluster role ordinally, so a verbatim-bound `Cluster-SITE-A`
yielded a null ClusterId — and a null ClusterId applies NO cluster filter, which
made #528's cross-cluster over-grant guard fail OPEN. IsClusterRole is now
case-insensitive; the ClusterId suffix keeps its authored spelling, because
normalizing it would silently rename the Akka role of a running deployment.
#533 — three string-identity rules in one authorization path. AclScopeRef used
default (ordinal) equality while the trie builder filtered ClusterId and hashed
principals OrdinalIgnoreCase; the deploy gate's cross-cluster check was ordinal
too, which was the worse half — it could reject a whole deploy over a ClusterId
case difference the runtime would have accepted. It only worked because resolver
and builder were handed the same NodeAcl instances.
The rule is now stated once, on AclScopeRef, and applied throughout:
identifiers (ScopeId, ClusterId, Principal) compare OrdinalIgnoreCase; path
segments compare Ordinal. AclScopePathResolver re-keys its own maps rather than
documenting a comparer contract for callers, so the two independent producers —
EF at the gate, artifact JSON on the node — cannot diverge from the outside.
PermissionTrieNode.Children stays Ordinal: those are browse names, and Speed /
speed are two nodes that can coexist under one device.
Split PermissionTrieBuildDiagnosticReason.RetiredScopeKind out of
UnresolvedScope. A legacy FolderSegment row and a typo'd id had the same message
and different remedies.
Falsified: five mutations, each reddening only its own guard — remove the
PostConfigure (3 tests), revert IsClusterRole to ordinal (3), revert AclScopeRef
equality (1), revert the resolver's map comparer (6), revert the gate's
comparisons (2), remove the FolderSegment short-circuit (1).
Test isolation: OTOPCUA_ROLES is process-global and xUnit runs classes in
parallel, so the three validator suites that now resolve effective roles share a
non-parallel collection. Without it, cases passing no roles would intermittently
observe another class's env var.
Full solution build clean; full suite green apart from two pre-existing failures
confirmed identical on
|
||
|
Joseph Doherty
|
3a6b2a1daf |
feat(acl): variable-depth scope model + the scopePaths producer (#520 stage 3)
Makes the /raw device tree addressable by an ACL rule, and builds the
scope-path producer PermissionTrieBuilder has accepted since it was written
but which had no caller anywhere.
The plan for this stage said "add NodeHierarchyKind.Raw, whose path is the
RawPath segments Folder/Driver/Device/TagGroup/Tag". That shape is wrong.
RawFolder.ParentRawFolderId and TagGroup.ParentTagGroupId are both
self-referencing, so folders and tag groups each nest without bound and a real
RawPath is folder…/Driver/Device/group…/Tag. NodeScope was fixed-arity and
PermissionTrie.CollectMatches was five hardcoded steps, so the scope model
could not address a real raw tree at all. NodeScope is now {ClusterId, Kind,
Segments} and the walk is a segment loop. The trie STRUCTURE was already
variable-depth (NodeAclPath.Segments is a list, Children is a recursive
dictionary), so only the walker and the scope record had to change.
Four decisions worth keeping:
- The namespace level became the realm. Level 1 gated on NodeScope.NamespaceId,
but v3 has no Namespace entity — the snapshot was retired when the address
space went dual-namespace, and the identifier appears nowhere in production
outside NodeScope. So the level could only ever fail to match, which silently
made every sub-cluster grant unreachable while cluster grants kept working.
It is now a realm token derived from the scope's Kind, not carried on it, so
the walk and the producer cannot disagree about its spelling.
- Provenance comes from the granting row's own ScopeKind, recorded on TrieGrant
at build time. Depth-derived provenance could not name a raw level at all,
since the depth of "Device" varies with folder nesting.
- The trie comparer moved from OrdinalIgnoreCase to Ordinal. Raw sibling-name
uniqueness is ordinal, so Speed and speed can both exist under one device and
a grant on either would have matched both.
- The null-scopePaths "deterministic test mode" is deleted. It planted
unresolvable rows at a direct child of the trie root, a position the walker
cannot reach, so the grant looked placed and could never match — and every
test using that mode proved nothing about production placement. scopePaths is
now required, and a row that does not resolve is dropped with a diagnostic.
A scope resolves to a LIST of paths: a raw tag projected into N equipments is
reachable at N+1 NodeIds and a write through any of them lands on the same
driver reference, so a Tag-scoped grant covering only the raw node would be
bypassable through a UNS one. The lookup is keyed on (ScopeKind, ScopeId), not
ScopeId alone — ScopeId has no FK and no type discriminator, so an id collision
across entity types would hand a grant the wrong node's paths.
Also closed here, all found by review rather than asked for:
- NodeAcl rows had no deploy-time validation of any kind and were not reachable
from DraftValidator at all (DraftSnapshot carried no NodeAcls). They are now
gated for dangling ids, cross-cluster scopes, blank/contradictory ScopeId, the
retired FolderSegment kind, and undefined ordinals.
- ConfigDbApiKeyStore conferred roles from Cluster-scoped grants authored in ANY
cluster, because it filtered on scope kind with no ClusterId predicate while
ArtifactApiKeyStore read an already-cluster-filtered snapshot. On a fused
admin+driver node that was a cross-cluster over-grant. Its doc comment claimed
the two stores matched.
- The two AdminUI ACL banners still said enforcement was LDAP-group -> role
mapping, which stages 1/2 made false in both directions.
- The /apikeys page was never classified in PageAuthorizationGuardTests, so that
test has been red since the key-minting commit merged — I did not run
AdminUI.Tests before merging stage 4.
Falsified rather than assumed: 8 mutations against the trie tests, 5 against the
deploy gate (including one proving a cross-cluster row is not silently
mis-diagnosed as unresolved), a staged red->green for the over-grant regression,
and the realm gate, the comparer and the malformed-path guard each broken
individually to confirm only their own tests go red.
Node ACLs are still not ENFORCED — nothing constructs a NodeScope or calls the
evaluator yet. That is stages 5 and 6; this stage makes the model able to
express what those will enforce.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
|