7a627d6c873426d2e30713501c5df55fd2fba815
1444 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Joseph Doherty
|
7a627d6c87 |
feat(mtconnect): driver factory delegating to the single config parser (Task 15)
The factory carries NO config DTO of its own: CreateInstance delegates to MTConnectDriver.ParseOptions, the single authority for the config document. A second DTO would drift from the parse the runtime's ReinitializeAsync path uses, and the drift would only surface at deploy time. Construction is connection-free (agentClientFactory: null) because the Wave-0 universal browser builds a throwaway instance per browse probe, and the returned instance is never narrowed — its five capability interfaces ARE the runtime's dispatch surface. Pinned by tests asserting all five plus the deliberate absence of IWritable. DriverTypeName is the literal "MTConnect"; Task 16 adds DriverTypeNames.MTConnect and the host registration that must equal it. |
||
|
Joseph Doherty
|
1f1ae174d9 | feat(mtconnect): IDriverProbe one-shot /probe reachability (Task 14) | ||
|
Joseph Doherty
|
a3a0cfe9bb |
fix(mtconnect): close the silent-refusal + teardown-wedge defects in the pump (Task 11 review)
C1 (critical): a latched "this endpoint cannot stream" verdict could end with a GREEN driver holding a permanently silent subscription. DriverInstanceActor handles a tag-set change as Unsubscribe-then-Subscribe with no re-initialize; the unsubscribe cleared the stream-degraded flag, the resubscribe was refused by the latch without a word, and one successful read then reported Healthy. StartSampleStreamCore now re-asserts the degradation and logs a Warning naming the remedy, and teardown no longer clears the flag while the latch is set. NOT promoted to Faulted: DriverHealthReport maps any Faulted driver to a /readyz 503, so that would de-ready the whole node over one misconfigured Agent whose reads are perfectly healthy. Faulted stays for the case that earns it. I1: teardown waits are bounded (5 s) and honour the caller's token. They were unbounded and ignored it, so DriverInstanceActor's 5 s budget was inert and PostStop — which blocks a dispatcher thread on ShutdownAsync while holding the lifecycle semaphore — could be wedged forever by one blocking subscriber. The cancellation source is disposed only when the loop is provably gone. Applied to StopProbeLoopAsync too: Task 13 reproduced the identical shape, and closing the class in one method while leaving the other as the pattern to copy is worse than not closing it. I2: the reconnect ladder was monotonic for the process lifetime, so ~9 unrelated drops pinned the driver at MaxBackoffMs forever. A stream that delivered before dropping now starts a fresh ladder (at attempt 1, not 0, so MinBackoffMs is still honoured). I3: MaxBackoffMs <= 0 clamped every delay to zero — an operator-authorable reconnect spin. Treated as unset, like a multiplier that cannot grow. I4: the session published instanceId without NextSequence, so after a restart it held the new agent's id beside the dead one's cursor, and a gap/OUT_OF_RANGE re-baseline (id unchanged) never published the cursor at all. Both move in one CAS, and the pump publishes its cursor as it exits. I5: OnDataChange was a multicast Invoke — the first throwing handler aborted the rest of the list, starving every later subscriber. Now walked by hand with the catch inside the loop. The test that claimed to cover this registered the recorder BEFORE the thrower, so it passed regardless; swapped, it was red. Faults are latched to one Warning per stream generation (Debug thereafter) so a consistently-throwing consumer cannot flood the log. Also: the pump restarts after an unexpected fault (IsCompleted, not null); a device-scope that matches nothing is a Warning, not a Debug tally; a device or component with neither name nor id is skipped rather than emitting a blank path segment; DiscoverAsync's remark no longer claims DriverInstanceActor retries (it does not for a Once driver, and injection is dormant in v3); and two flake seeds in the fake are gone (Dispose re-read its own counter; _disposeCts was disposed under a racing SampleAsync). 439/439. Every changed behaviour falsified by mutation. |
||
|
Joseph Doherty
|
a92584c45e | feat(mtconnect): host-connectivity probe + instanceId rediscover (Task 13) | ||
|
Joseph Doherty
|
2732081daa | feat(mtconnect): ITagDiscovery streams tree + SupportsOnlineDiscovery opt-in (Task 12) | ||
|
Joseph Doherty
|
a41c26b5de |
feat(mtconnect): ISubscribable /sample pump + ring-buffer re-baseline (Task 11)
One shared /sample long poll behind every subscription handle. Subscribe fires initial data per reference from the primed /current (Part 4 convention) and returns without waiting on the stream; the pump runs on its own task under its own token — never the caller's Subscribe token, which is disposed the moment that call returns. The pump owns every way the sequence can break: - sequence gap, measured against the RUNNING cursor (the previous chunk's nextSequence). Comparing against the opening `from` reports a gap on every chunk once the buffer rolls — a /current re-baseline storm. - OUT_OF_RANGE under HTTP 200 (InvalidDataException out of SampleAsync), which IsSequenceGap cannot see at all — without it a driver recovers from falling a little behind but not from falling a lot. - agent restart, checked BEFORE the gap because a restart resets sequences and so usually trips the gap check too; it clears the index (the held values describe a device model that no longer exists) and tells the subscribers. - every chunk advances the cursor, heartbeats included. Re-baseline calls CurrentAsync directly on the client the pump already holds: routing it through ReinitializeAsync would take the non-reentrant lifecycle semaphore from inside a pump a lifecycle method may already be awaiting, and hang the driver with no exception and no log. StopSampleStreamAsync is filled in (same call sites) and InitializeAsync now stops the stream before teardown too, so a re-Initialize on a live instance cannot leave a pump enumerating a disposed client. MTConnectStreamEnded/TimeoutException reconnect under a geometric backoff with a 100 ms growth floor (MinBackoffMs defaults to 0, and 0 x multiplier is still 0). MTConnectStreamNotSupportedException does NOT retry — it latches, reports loudly, and is cleared only by a re-initialize. Health precedence is explicit: /current and /sample fail independently, so each path owns a flag, clears only its own, and Healthy requires both clear. Neither can paper over the other's failure. 29 new tests, all deterministic — no sleeps, no wall-clock assertions. The fake grew per-generation sample streams (so a reconnect has somewhere to land), scripted /current answers, scripted sample failures, and a chunk-consumed signal that also fires when the consumer abandons the stream. 375/375 green. |
||
|
Joseph Doherty
|
26eb511a7e |
fix(mtconnect): review remediation for the driver lifecycle (Task 9)
Important 1 - the probe-model triple was read outside _lifecycle while every
write happened under it. Packed (Client, ProbeModel, ProbeModelDataItemCount)
into one immutable AgentSession swapped by a single Volatile.Write, matching
what _index already does, rather than locking the readers: both await the
network, so taking _lifecycle would stall a shutdown for a whole
RequestTimeoutMs and would widen the non-reentrancy hazard. GetOrFetch and
Flush re-publish via CompareExchange so a late re-cache cannot undo a
concurrent teardown/re-init, and a client disposed mid-fetch now surfaces as
the same "not connected" InvalidOperationException browse already handles
instead of a raw ObjectDisposedException.
Important 2 - HasConfigBody decided emptiness by matching the literals "{}" /
"[]", so "{ }", "{\n}" and every pretty-printed empty document fell through to
ParseOptions, failed the required-AgentUri check, and turned a semantically
empty config into a cold-start fault. Now parsed: an object/array with no
elements (or a bare null) is empty; malformed text is deliberately NOT empty so
ParseOptions produces the real quoted error rather than silently starting on
stale options.
Important 3 - documented the _lifecycle non-reentrancy hazard in the code, on
both the semaphore and StopSampleStreamAsync, naming Task 11's re-baseline as
the specific path that would deadlock and giving the CurrentAsync-directly
pattern that avoids it.
Important 4 - removed the Initialize/Reinitialize asymmetry rather than
documenting it. Both now share one rule via ResolveIncomingOptions: an
unreadable config document never destroys working state, and faults only a
driver that had none. InitializeAsync previously tore down before parsing, so a
bad document on a live instance destroyed a healthy client - the exact outcome
ReinitializeAsync was written to avoid.
Minors: SafeDispose traces the swallowed disposal fault at Debug (a client that
cannot be released is how a handle leak starts); the RequirePositive test is a
Theory over all four timing knobs, not just RequestTimeoutMs (arch-review
01/S-6); the footprint test no longer overclaims "is zero before initialize".
CannedAgentClient gains a one-shot ProbeGate so a lifecycle change landing
mid-request is deterministic - no timers. 346/346 (329 + 17). Six mutations
verified: fetch-CAS, disposed-translation, flush retired-session guard,
literal HasConfigBody, teardown-before-parse, and two dropped RequirePositive
calls each fail only their own tests.
|
||
|
Joseph Doherty
|
0dfc3e1077 |
feat(mtconnect): IReadable via /current, ordered per-ref snapshots (Task 10)
One deadline-bounded /current per batch, answered out of that one whole-device document: N references cost one round-trip, one snapshot each, in request order, duplicates included. DEVIATION (deliberate, from the plan's implied shared-index write): the response is indexed into a PER-CALL snapshot index, never into the driver's shared observation index. That index is written by Task 11's /sample pump and is last-write-wins with no timestamp arbitration; a read folding its /current into it would be a second writer racing the first, and /current is frequently the OLDER document (the Agent composes it while the pump's newer delta is in flight). Losing that race rolls a subscribed value backwards and leaves it there until the data item next changes. Both paths coerce through the same MTConnectObservationIndex logic, so a read and a subscription report a given observation identically -- the read path is simply a pure function of (document, authored tags). Failure posture is the inverse of InitializeAsync: nothing but genuine caller cancellation crosses the capability boundary. Unreachable Agent / blown deadline / unusable answer => every ref codes BadCommunicationError and the driver degrades (never downgrading Faulted); no client at all (pre-initialize, faulted, post-shutdown) => BadNotConnected with health untouched. The index's BadWaitingForInitialData / BadNodeIdUnknown / BadNoCommunication / BadNotSupported distinctions pass through unmodified. 21 tests pinning arity, order, duplicates, empty batch, one-round-trip, each Bad code, read-time freshness, the anti-clobber invariant, the deadline, and cancellation-propagates-but-agent-failure-does-not. CannedAgentClient gains a cancellation-observing CurrentGate (still no timers or sleeps of its own). Test csproj takes the AbCip/S7 OTOPCUA0001 NoWarn -- this suite calls the capability directly by design. 329/329 green. |
||
|
Joseph Doherty
|
b79df65d3e |
feat(mtconnect): driver shell IDriver lifecycle over agent-client seam (Task 9)
MTConnectDriver implements IDriver only: connection-free ctor, initialize (deadline-bounded /probe + /current, caching the device model, the agent instanceId, and a primed observation index), reinitialize, shutdown, health, footprint, and cache flush. Decisions worth knowing: - The driverConfigJson argument is HONOURED, not ignored. DriverInstanceActor delivers a config change only via ReinitializeAsync(json) on the live instance, so a driver reading only its ctor options would turn every MTConnect config edit into a silent no-op that still sealed the deployment green. ParseOptions lives on the driver; Task 15's factory must delegate to it rather than deserialize a second DTO. - /probe ok but priming /current failing is Faulted, not Healthy-with-no-data: the index IS the read surface and the /sample cursor comes from /current. - ReinitializeAsync rebuilds the client whenever ANY option the client reads at construction changed - AgentUri, DeviceName, RequestTimeoutMs, HeartbeatMs, SampleIntervalMs, SampleCount - not just the first two. The timing knobs are baked into the deadlines, the watchdog window, and the /sample query string. - An unparseable config faults a cold start but leaves a RUNNING driver serving its previous valid configuration (the deployment fails instead). - IMTConnectAgentClient now extends IDisposable rather than the driver doing `as IDisposable`, which would silently no-op against a non-disposable fake and leak an HttpClient + socket handler per re-deploy. Sync, not async: every resource behind the seam is sync-disposable, and the async unwind belongs to the SampleAsync enumerator the pump owns. - Flush drops the probe/browse cache and keeps the index; every model consumer goes through GetOrFetchProbeModelAsync so a flush cannot wedge browse. Tests: CannedAgentClient (shared, deterministic - channel-backed scripted /sample with no timers, call counts, disposal tracking, failure injection) + 27 lifecycle tests. 308/308 in the project. |
||
|
Joseph Doherty
|
3f8f3e1863 |
fix(mtconnect): deterministic concurrency test + DATA_SET -> BadNotSupported
The concurrency test asserted a post-Apply post-condition from inside the race window: dev1_pos is authored, so a reader beating the first Apply correctly observes BadWaitingForInitialData, which the test's two-member "legal" set omitted. Consistently red (10/10) rather than intermittent. Rewritten to assert only properties that hold at every instant — the whole (status, value, source timestamp) triple against the three legal snapshots, which is the real torn-snapshot check — plus a read counter guarding a vacuous pass and one deterministic closing Apply for an exact end state. The index is unchanged here; BadWaitingForInitialData is a designed state. Also codes structured (DATA_SET / TABLE) observations BadNotSupported now that the parser supplies MTConnectObservation.IsStructured, so a two-entry data set no longer surfaces as the Good string "12". |
||
|
Joseph Doherty
|
712635ce8c |
fix(mtconnect): make a /sample stream end loud, and correct the gap contract (Task 7 review)
Code review of c46540ae approved the framing algorithm but moved the risk onto the contract around it. C1 (critical). SampleAsync returned normally on three different events: EOF, the closing --boundary--, and a response that was not multipart at all (where it yielded one parsed snapshot and finished). The seam documents the stream as yielding until ct is cancelled, so a Task 11 pump written to that contract would silently drop its subscription or hot-loop reconnecting; the non-multipart leg reported a configuration error as a healthy finished stream, which is the #485 quiet-successful-termination shape aimed at the very next task. Every non-cancellation end now throws: MTConnectStreamEndedException (ConnectionClosed / ClosingBoundary - transient) or MTConnectStreamNotSupportedException (configuration - reconnecting reproduces it forever), sharing one catchable base. The non-multipart body is still parsed first so an Agent's own MTConnectError text wins, but the document is NOT yielded. I1. IsSequenceGap moved to IMTConnectAgentClient (static) and its first parameter is now expectedFrom. The old name and doc were wrong for every chunk after the first - the correct comparand is the PREVIOUS chunk's NextSequence, and comparing against the opening "from" argument reports a gap on every chunk once the ring buffer rolls, i.e. an endless /current re-baseline storm. Both doc blocks also now record that cppagent answers from < firstSequence with an OUT_OF_RANGE MTConnectError under HTTP 200, which surfaces as InvalidDataException and NOT as a gap - so Task 11 must treat that parse failure as a re-baseline trigger too. I2. Every multipart test served the whole body from one buffer, so every framing test completed in a single ReadAsync - the split-boundary path, the split-header path and the multi-fill loop were correct by inspection only. A ChunkedStream double (N bytes per read, N = 1/3/7) now drives the fixtures through a split transport, plus a >16 KB document that forces a buffer resize. Falsifiability: removing either scan overlap, or collapsing the fill loop, fails ONLY the new chunked tests. I3. Disposal mid-enumeration surfaces as OperationCanceledException via an internal dispose-linked token, not a raw ObjectDisposedException that Task 9's re-init would inflict on an enumerating pump. I5. HeartbeatMs, SampleIntervalMs and SampleCount join RequestTimeoutMs in construction-time positive-value validation. All three reach the query string and an Agent answers heartbeat=0 with an HTTP-200 MTConnectError that names no config key. I4/M2. The no-Content-length framing fallback logs a one-shot warning (the client takes an optional ILogger); a multipart response with no boundary parameter fails fast instead of degrading into a watchdog timeout. M5/M6. Shared element/attribute reading rules extracted to MTConnectXml; the probe parser documents why it alone does not trim BOM/whitespace. The literal U+FEFF in source became a '' escape. Also, from Task 8: MTConnectObservation gained IsStructured (default false), set from element.HasElements. A DATA_SET/TABLE observation's <Entry> children concatenate through element.Value to nonsense ("12" for two entries) that the index would publish as Good, and only the parser can tell that apart from a legitimate space-bearing Message. False for CONDITION observations - their value comes from the element name, so child content cannot corrupt it. 275/275 green in this suite's own files. |
||
|
Joseph Doherty
|
2399d075e9 | feat(mtconnect): observation index + UNAVAILABLE->BadNoCommunication (Task 8) | ||
|
Joseph Doherty
|
9ebefa67a8 |
feat(mtconnect): current/sample parse + sequence-gap detection (Task 7)
Adds the /current and /sample legs to MTConnectAgentClient: - MTConnectStreamsParser: hand-rolled System.Xml.Linq parse of an MTConnectStreams document into MTConnectStreamsResult. Observations are every element child of <Samples>/<Events>/<Condition>, keyed by dataItemId (the element NAME is the data item's type in PascalCase, so a fixed name set would drop observations). A CONDITION's value is its element name, not its text - <Normal/> is empty - and <Unavailable/> normalizes onto the same UNAVAILABLE sentinel a Sample/Event carries as text, so Task 8's no-comms mapping sees one token. Timestamps are normalized to DateTimeKind.Utc explicitly. - IsSequenceGap(requestedFrom, chunk) => chunk.FirstSequence > requestedFrom, so Task 11's pump can re-baseline after a ring-buffer overflow. Equality is NOT a gap. - SampleAsync: ResponseHeadersRead + a hand-rolled multipart/x-mixed-replace frame reader (Content-length fast path, boundary-scan fallback), bounded by a heartbeat watchdog derived from HeartbeatMs so a silent Agent faults instead of wedging the pump - the S7 frozen-peer shape (arch-review R2-01). The long poll gets its own HttpClient with an infinite Timeout so the unary /probe + /current deadline stays bounded. Task 0 package decision, option (c): both TrakHound PackageReferences and their PackageVersions are dropped. MTConnectHttpClientStream takes a URL and dials its own socket (no handler/stream seam, so it cannot serve a socket-free unit suite), and it emits already-parsed documents through the formatter lookup Task 6 proved is missing - framing-only reuse was never actually on offer. The driver now depends on nothing but the BCL. Recorded in the plan's Task 0 CORRECTION block. Also folds in two Task 6 review carry-overs: the stale IMTConnectAgentClient doc comment claiming a TrakHound-backed implementation, and a probe test constructing a mixed-namespace vendor extension (<x:Pump> with default-namespace <DataItems> children) that backs the parser's ignore-the-namespace claim with a test. 187/187 green. |
||
|
Joseph Doherty
|
35174d2f5e | feat(mtconnect): agent client /probe parse to device model (Task 6) | ||
|
Joseph Doherty
|
29863f3947 | fix(mtconnect): options carry the authored tag definitions (Task 2 follow-up) | ||
|
Joseph Doherty
|
bfab95f344 | fix(mtconnect): infer numeric EVENT types from the probe's UPPER_SNAKE spelling (Task 3 follow-up) | ||
|
Joseph Doherty
|
817cc9d729 | test(mtconnect): canned probe/current/sample XML fixtures incl. forced-gap (Task 4) | ||
|
Joseph Doherty
|
93f2501552 | feat(mtconnect): pure data-type inference table + golden test (Task 3) | ||
|
Joseph Doherty
|
34cc1825ad | feat(mtconnect): driver options + tag definition records (Task 2) | ||
|
Joseph Doherty
|
fe96e7b8c2 | build(mtconnect): scaffold Contracts+Driver+Tests projects, add to slnx (Task 1) | ||
|
Joseph Doherty
|
123ddc3f40 |
fix(scripteditor): complete + hover absolute RawPaths (#490)
The issue reports a missing feature. The cause is a stale projection, and the
consequence was worse than the gap described.
`ScriptTagCatalog` still emitted `Tag.TagConfig.FullName` — the pre-v3 driver wire
address. Since v3 the mux key is the **RawPath**: `AddressSpaceComposer` harvests the
`ctx.GetTag("…")` literals into `DependencyRefs`, those become `VirtualTagActor`
dependency refs registered with `DependencyMuxActor`, whose `_byRef` is keyed Ordinal
by `AttributeValuePublished.FullReference` — and a driver's wire ref IS the RawPath
(`DriverHostActor._nodeIdByDriverRef` is keyed `(DriverInstanceId, RawPath)`). The
`{{equip}}/<RefName>` form already substitutes to RawPaths at both compose seams.
So the catalog was wrong in BOTH directions, not merely incomplete: completion
offered paths that could never resolve at runtime, while a CORRECT absolute RawPath
got no completion and hovered as "not a known configured tag path". The file's own
comment admitted the raw/UNS catalog was left for "Batch 2/3" and it never came.
Now projected through the shared `RawPathResolver` — the same byte-parity authority
`DraftValidator` and `AddressSpaceComposer` use — so a suggested path is identical to
the one the deploy gate and the runtime compute. A tag with a broken ancestry chain
resolves to null and is omitted: the runtime could not route it either, so offering
it would be a lie. Hover additionally gains the owning `DriverInstanceId`, which the
topology now makes available (it was always null before).
Editor-accepts <=> publish-accepts is preserved: no diagnostic keys off this catalog
(`OTSCRIPT_EQUIPREF` only marks `{{equip}}` paths), so this changes completion and
hover only — it cannot newly reject a script.
The existing tests pinned the v2 contract and were re-authored: a RawPath only exists
if the RawFolder -> DriverInstance -> Device -> TagGroup -> Tag chain does, so the
seed now builds one. Added coverage for group-nested paths, a driver at the cluster
root (no folder segment), a broken chain being omitted rather than guessed, and the
pre-v3 FullName no longer resolving.
Live-gated on docker-dev (rig rebuilt, both central nodes):
- completion, empty literal -> every RawPath incl. group-nested
`opcua1/plc/OpcPlc/Telemetry/Basic/AlternatingBoolean`
- completion, prefix `pymod` -> `pymodbus/plc/HR200X`, `pymodbus/plc/ImportedTag`
- hover `pymodbus/plc/HR200X` -> "Tag · Type UInt16 · Driver MAIN-modbus"
- hover an unknown path -> "Not a known configured tag path"
AdminUI 742/742; script-analysis 55/55; solution builds 0 errors.
|
||
|
Joseph Doherty
|
0f38f48679 |
merge: Modbus RTU-over-TCP transport (#495)
Wave 1 of the driver-expansion program. The branch was complete and live-gated but had never been merged; it sat 15 commits ahead and 50 behind master. Purely additive behind the existing IModbusTransport seam — the application protocol (function codes, codecs, planner, coalescing, write path, probe, materialisation, HistoryRead) is identical across TCP and RTU; only the wire framing differs ([addr][PDU][CRC-16], no MBAP, no TxId). Zero changes to codecs/planner/health. Selected by a new Transport config field (ModbusTransportMode: Tcp | RtuOverTcp), routed through ModbusTransportFactory (throws on unknown mode), with the string-enum guard on options/DTO/factory and a Transport selector on the AdminUI Modbus form. ModbusSocketLifecycle was extracted from ModbusTcpTransport (behaviour-preserving) and is composed by both transports. Descoped by design: direct-serial (System.IO.Ports) and Modbus ASCII — RTU buses are reached exclusively via a serial->Ethernet gateway. No per-tag config changes; the existing per-tag UnitId already makes a multi-drop bus work. Re-validated against current master at merge time (the branch's own gate predates 50 commits of master): no overlapping files, clean merge, solution builds 0 errors, and Modbus 344/344, Modbus.Addressing 161/161, AdminUI 740/740, Runtime 507/507, Configuration 131/131 all pass. Confirmed the Transport selector is still reachable through master's current authoring path (DriverConfigModal -> ModbusDriverForm), so the branch's live /run gate result still applies. |
||
|
Joseph Doherty
|
4b9bcddbcc |
feat(drivers): periodic reconcile of desired vs actual subscriptions (#488)
Option A from the issue — the cheap consistency check, no interface change. The desired ref set was re-applied on a Connected TRANSITION (`ResubscribeDesired`) and on a deploy's `SetDesiredSubscriptions`, and nowhere else. A subscription lost while the driver STAYED Connected — a subscribe that failed and was never retried, a handle dropped down an error path — left the actor subscribed to nothing while still reporting Healthy, indefinitely. `ReconcileSubscription` now rides the existing 30 s `health-poll` tick: while Connected, a driver holding a desired set but no live handle is re-subscribed. Gated on `ISubscribable`. A driver that cannot subscribe at all can still be handed a desired set, and without the guard it would be told `Subscribe` every tick and fail every tick, forever. This is a state-machine consistency check, NOT a probe — it can only see that this actor holds no handle, never that a handle is present but dead server-side, because `ISubscriptionHandle` is opaque. That is option B, and it stays deferred until a real occurrence shows the handle outliving the subscription; it would need a liveness member implemented across all eight drivers with different subscription mechanics. A redundant `Subscribe` is possible (a tick queued ahead of an already-queued one observes the same no-handle state) and is harmless: `HandleSubscribeAsync` is a `ReceiveAsync`, so no tick is processed mid-subscribe, and it drops any prior subscription before establishing the new one. Suppressing it would need a pending-subscribe flag threaded through every self-tell site — more state than the race is worth. `healthPollInterval` becomes an optional Props parameter, mirroring the existing `rediscoverInterval` seam, so a test can drive the reconcile without waiting 30 s. Three tests. The reconcile itself is proven by a stub whose FIRST subscribe throws: nothing else would ever retry it, so a second attempt can only come from the reconcile — disabling `ReconcileSubscription` turns it red. The two guard tests are absence assertions and are bounded by a positive control rather than a sleep: a counting health publisher makes ticks PROCESSED observable, so "still one subscribe" means the reconcile ran and declined, not that the timer had not fired. Explicitly not deploy-time drift (#486) — re-asserting an already-correct desired set would not have helped there. |
||
|
Joseph Doherty
|
2193d9f2e4 |
fix(config): refuse to store a Sql credential in a node config override (#499)
`ClusterNode.DriverConfigOverridesJson` is the third surface a Sql driver's config is persisted on, and the only one nothing gated. It is a map keyed by `DriverInstanceId` merged onto the cluster-level `DriverConfig`, so a literal `connectionString` pasted there leaks exactly as one pasted into the driver — the leak #498 closed for `DriverInstance.DriverConfig` and `Device.DeviceConfig`. Gated at the SAVE, not at the deploy — a deliberate departure from both options the issue offered: - `DraftSnapshot` carries no `ClusterNode` rows, and adding them would widen the snapshot and every builder of one for a single rule about a value that never enters the artifact. - More to the point, a deploy gate is the wrong instrument here. Node overrides are not in the artifact, so blocking a deploy would not stop the credential being stored — it is already in the database and replicated by then. Refusing the save is the only point where "refuse to store it" is literally true, which is #498's own framing: discarding a secret on read is not the same as refusing to store it. The check moves into a shared `SqlCredentialGuard` so the two enforcement points cannot drift; `DraftValidator` now calls it instead of its own private helper. Kept deliberately narrow — the `Sql` driver's `connectionString` only, and only for instance ids that ARE Sql drivers. The issue asked whether to generalise to credential-shaped keys across every driver type; not doing that, for the same reason #498 did not: a broader sweep would start refusing configs that are legitimate today for drivers which never made an indirect-credential guarantee, which is a regression rather than defence in depth. Widen per driver, as each gains its own contract. The error names the offending driver instance(s) and NEVER the value — it reaches the AdminUI and the audit trail. 14 new cases cover both halves, including the case variants (System.Text.Json binds `ConnectionString` to `connectionString`, so a case variant is the same key, not a bypass), non-Sql ids being ignored, and every blank/malformed/non-object shape. |
||
|
Joseph Doherty
|
240d7aa8aa |
fix(test): give the three absence assertions a positive control (#501)
`AwaitAssert` returns on its FIRST success, so an assertion that is already true at t=0 passes instantly and spends none of its duration. All three sites asserted an absence against a collection that starts empty, so none of them ever gave the handler the time their comments claimed. Replaced with mailbox-ordering positive controls rather than sleeps: - `RouteNativeAlarmAck_unknown_node_is_dropped` — follow the unmapped ack with a MAPPED one. The host forwards via `Tell` and the child handles `RouteAlarmAck` with `ReceiveAsync` (which suspends its own mailbox), so once the control reaches the driver the unmapped one has provably been handled. - `RouteNativeAlarmAck_on_non_primary_is_dropped` — the role itself is the control: return the node to Primary and re-send. Comments (`gated` / `control`) are the discriminator, so a leaked ack is visible rather than merged into a count. - `PeerProbeSupervisorTests.Single_node_snapshot_spawns_no_children` — a FOURTH site the issue did not list. It says the other `ChildCount.ShouldBe(0)` waits are each preceded by a `ShouldBe(1)`; that is true of three of the four, but not this one, which asserts the initial state. Now followed by a two-node snapshot: the count reaching 1 proves the single-node snapshot was processed, and a local-node child would have made it 2. The issue warned these may turn red, which would be a real defect surfacing. They did not — the routing is correct. Proven by breaking it deliberately instead: disabling the Primary gate reddens the Secondary test, and routing unmapped ids to an arbitrary mapping reddens the unknown-node test. Under the old form both breaks passed. |
||
|
Joseph Doherty
|
95295210b0 |
fix(test): scope the EventPump counter capture to its own pump (#503)
Root cause was cross-test measurement leakage, not a timing budget. EventPump's `Meter` is STATIC — one instance per process — and a `MeterListener` can only select by *meter* in `InstrumentPublished`. `StartMeterCapture` therefore received measurements from every `EventPump` alive anywhere in the assembly. xUnit runs test classes in parallel and several build pumps (`EventPumpStreamFaultTests`, the `Driver-X` test in this same file), so a sibling's events landed in these counters. That is why the flake needed a busy box: it needed two tests to overlap. Measured directly during the gate — `Received = 11` in a run that emitted 10. The old `Received == Dispatched + Dropped + InFlight` assertion is what turned the leak into a failure. `InFlight` is DERIVED as `Received - Dispatched - Dropped`, so the identity is a tautology that cannot fail on a real defect — only on a foreign increment landing between its four separate `Interlocked.Read` calls. Hence the reported subject, `counters.Received`. It is deleted, along with the `InFlight` member, and replaced by direct assertions on the three MEASURED counters. Three further defects in the same test, all found on the way: - **The "held" dispatch loop was never held.** `OnDataChange` is `EventHandler<T>` — void-returning — so `async (_, _) => await gate` is async VOID: it returned to the dispatch loop at the first await and blocked nothing. Measured `Dispatched = 2..3`, never 0. Drops happened only when the producer happened to outrun the consumer, so the arrangement the doc comment describes was fiction and the assertions rode on scheduling luck. Now a synchronous `ManualResetEventSlim.Wait()` on the loop's own thread, pinned by `Dispatched.ShouldBe(0)`. - **Fixed `Task.Delay(150)`** replaced by a presence poll on the drop count (#500's rule). Dropped starts at 0, so it cannot be satisfied by the initial state. - **Emission is staged** — one event, wait for the dispatcher to enter the handler, then the other nine — so the arithmetic is exact (`10 - 1 held - 2 buffered = 7`) rather than scheduler-dependent. The gate release moved into `finally`, ahead of disposal: `DisposeAsync` awaits the dispatch loop, which is now genuinely parked on that gate, so a failed assertion would otherwise hang instead of failing. Verified: 15/15 clean under the exact contention the issue measured (Runtime + AdminUI + Core concurrently) — where the half-fixed version still failed on run 4. Falsifiability: restoring the async-void handler turns the new assertions red. |
||
|
Joseph Doherty
|
47d148daf9 |
fix(adminui): never slice a DB-sourced string with the range operator (#504)
`Scripts.razor` rendered a hash prefix as `@s.SourceHash[..12]…`. `SourceHash`
is a plain nvarchar(64) with no length floor, so any value shorter than 12
characters threw ArgumentOutOfRangeException out of BuildRenderTree — unhandled
in Blazor, which takes the WHOLE page to an HTTP 500, not just the offending
row. Live-reproduced on docker-dev, where two Script rows carry `h1` and
`h-abs-hr200`: /scripts was a hard 500.
New `Components/Shared/DisplayText.Abbreviate(string?, int)`:
- null/blank renders the Admin UI's em-dash placeholder rather than throwing;
- a value already within budget renders verbatim with NO ellipsis, so the
display never implies text that isn't there (the old markup appended "…"
unconditionally, outside the slice — `h1` would have shown as `h1…`);
- a maxLength < 1 is clamped, so a display bug cannot become a page crash.
Applied to every unguarded range-operator slice over a DB-sourced string found
by sweeping the AdminUI for `[..N]`:
Scripts.razor SourceHash (the reported crash)
Certificates.razor Thumbprint (read off the on-disk store)
Deployments.razor x2 RevisionHash
Clusters/ClusterOverview.razor RevisionHash
The other `[..N]` hits are safe and untouched: Guid.ToString("N") slices
(always 32 chars) and the `Length > 60 ? [..60]` ternaries that self-guard.
NOTE — the issue text blamed the docker-dev seed; that was wrong and is
corrected on #504. `seed-clusters.sql` inserts only ServerCluster, ClusterNode
and LdapGroupRoleMapping. The short-hash rows came from earlier live-gate
authoring. A freshly seeded rig is fine; the trigger is any Script row that did
not go through ScriptEdit / UnsTreeService.HashSource — REST-API authored,
hand-inserted, or migrated in.
Tests: 14 new in DisplayTextTests, covering the short/null/blank/clamped cases
and a never-throws sweep over every value x budget combination. AdminUI suite
739/739.
Live-verified on docker-dev (AdminUI has no bUnit): /scripts 500 -> 200 across
both central nodes, rendering `hash=h1` and `hash=h-abs-hr200` in full;
/deployments still truncates 64-char hashes at 12 with the ellipsis;
/certificates and /clusters/MAIN unchanged.
|
||
|
Joseph Doherty
|
549c656489 |
fix(alarms): re-assert non-normal scripted-alarm conditions after a (re)load (#487)
A deploy that triggers a FULL address-space rebuild clears `_alarmConditions` and re-materialises every condition node fresh — inactive, acked, confirmed, unshelved. The engine reloads its persisted state and re-derives Active from the predicate, so it ends up correct; but there is no transition to report, so `LoadAsync` yields `EmissionKind.None` and `OnEngineEmission` drops it. Nothing writes the node. Live-reproduced on the docker-dev rig against the pre-fix image: an alarm that had fired and cleared but was still UNACKNOWLEDGED came back from a rebuild reporting Acknowledged with Retain=false — so it disappeared from ConditionRefresh entirely, while the engine still held Unacknowledged. An operator's outstanding alarm silently vanishes from the alarm list on deploy. `ScriptedAlarmHostActor.ReassertConditionNodes` closes it. After each load it reads the new `ScriptedAlarmEngine.GetProjections()` and sends one `AlarmStateUpdate` per alarm NOT in the Part 9 no-event position. Load-bearing properties: - Node-only. It sends `AlarmStateUpdate` and nothing else — never the `alerts` topic, never the telemetry hub. `alerts` is the historization path, so a row per alarm per deploy would append a duplicate historian/AVEVA record every time anyone deploys. This is why it reads a projection rather than re-emitting: `ScriptedAlarmProjection` carries no `EmissionKind`, so there is nothing for the emission machinery — or a future refactor — to mistake for a transition. The issue's sketch said `GetState(alarmId)` + `LoadedAlarmIds`; that alone would have clobbered the node's severity and message, so the projection also carries the definition's severity, the resolved message template, and the last-observed worst-input quality. - Only non-normal alarms. One in the no-event position already matches what the materialise built, so a never-fired alarm stays byte-identical to a deploy without this pass (including its Message, which the materialise seeds with the display name), and an all-normal deploy writes nothing at all. - Timestamp is the condition's own `LastTransitionUtc`, not the deploy instant. - No duplicate Part 9 events: `WriteAlarmCondition`'s delta-gate compares against the node's CURRENT state, so a re-assert onto a freshly materialised node is a genuine delta (fires once — correct, the rebuild reset what clients see) while one onto a surgically-preserved node is suppressed. Tests: 6 new host tests + 6 new engine tests. Falsifiability checked by disabling the call — 4 of the 6 host tests go red; the other 2 are absence guards against an over-broad fix and pass either way by design. Live gate (docker-dev, central-2, pre-fix image vs. rebuilt image): - pre-fix: condition absent from ConditionRefresh after a rebuild; - post-fix: present, Unacknowledged, Retain=True, stamped with its own LastTransitionUtc rather than the restart instant; - /alerts stayed empty across two re-asserts, with the panel proven live by a real ACTIVATED transition immediately afterwards — no duplicate history. |
||
|
Joseph Doherty
|
154171f48c |
test(runtime): fix the intermittent Runtime.Tests failure — 4 ordering/logic defects + the presence-budget class (#500)
The reported symptom was "Runtime.Tests occasionally reports Failed: 1" with no test
name. Instrumenting 30 full-assembly runs reproduced it at 13% (4 runs) and showed it
was never ONE flaky test: five failures across three distinct tests in that batch, and
five more distinct tests over the verification rounds that followed.
Two hypotheses were tested and DISPROVED by measurement before anything was changed:
- Cluster-formation timeout. Every test in this assembly forms a real single-node Akka
cluster over TCP in RuntimeActorTestBase's constructor and waits 5 s for MemberStatus.Up
— 219 formations per run. Instrumented: idle max 960 ms, under-load max 1470 ms, zero
timeouts. A 3.4x margin; not the cause.
- Ephemeral-port exhaustion from that bind churn. TIME_WAIT measured at ~0 throughout.
Not the cause.
FOUR GENUINE ORDERING/LOGIC DEFECTS (none is a timeout):
1. VirtualTagHostActorTests.ApplyVirtualTags_respawns_child_when_plan_changes_in_place
The test loops to accept RegisterInterest/UnregisterInterest in EITHER order — and
then asserts on mux.LastSender, which DEPENDS on that order. Under the [Register,
Unregister] interleaving LastSender is the DYING child and the assertion fails. Now
captures the sender of the Register message as it arrives. Fully deterministic; no
timing involved. (Swept the other four LastSender sites: all drain the Unregister
first, so their ordering is fixed. Only this one was unsafe.)
2. DriverHostActorWriteRoutingTests.Primary_routes_write_via_uns_nodeid_to_driver_by_rawpath
ApplyAck marks the end of the APPLY, not the point a write can succeed: the child is
still in Connecting, which deliberately fast-fails writes ("driver not connected"),
and the NodeId->driver reverse map has not been pushed. Now retries until accepted.
This does not weaken the assertion — every rejection branch replies WITHOUT reaching
the driver, so Writes.Count.ShouldBe(1) still means exactly what it did.
3. HistorianAdapterActorTests.Redundancy_snapshot_without_local_node_...
One 500 ms constant served both presence budgets (AwaitAssert) and absence windows
(ExpectNoMsg). Different quantities that happened to share a number, so the presence
budget could not be raised without slowing every absence check. Split into
AssertTimeout (5 s) and Settle (500 ms).
4. ContinuousHistorizationRecorderTests.Retry_after_writer_failure_eventually_acks
Waited on `outbox == 0`, which is ALSO true before the first append — so the poll
could win the race against the recorder and return having observed the INITIAL state,
after which the CallCount>=2 check outside the block failed against a recorder that
had not run. Both conditions are now polled together with the retry count as the
discriminator. (The preceding test in the same file documents this exact trap.)
THE PRESENCE-BUDGET CLASS:
After those four, three consecutive 30-run rounds each still failed once — on a
DIFFERENT test, in a DIFFERENT file, every time (OpcUaPublishActorApplyFailureTests 2 s,
OpcUaPublishActorRebuildTests 2 s, OpcUaPublishActorTests 500 ms). That is a
distribution, not a defect, and fixing it one test at a time was the wrong method.
RuntimeActorTestBase now carries a documented PresenceBudget (15 s) and 57 sub-3 s
AwaitAssert/AwaitCondition durations across four files route through it.
Why this is safe rather than sloppy: a presence budget is an upper bound before giving
up, not a wait — these helpers poll and return the instant the condition holds. Raising
one costs nothing on the happy path and CANNOT make a genuinely failing assertion pass;
it only changes how quickly a real breakage reports. Absence windows are the opposite
(the elapsed time IS the assertion) and were deliberately left untouched with their own
short, individually-calibrated literals.
ScriptedAlarmHostActorTests is a special case, diagnosed rather than merely raised: its
8 s budget was sized for message passing but actually waits on a REAL Roslyn compile
(ApplyScriptedAlarms -> ScriptedAlarmEngine.LoadAsync compiles every predicate). Cold
script compiles are the slowest and most variable thing in the assembly, so that class
gets 30 s with the reason recorded.
Verification: 30 consecutive full-assembly runs, 30 passed / 0 failed, against a 13%
baseline. Full solution builds; all 41 unit-test projects green.
NOT fixed here, filed as #501: DriverHostActorNativeAlarmAckRoutingTests:90 and :116 use
AwaitAssert for an ABSENCE assertion, so they return instantly and prove nothing. They
are excluded from the budget change on purpose — a bigger number does not fix them, and
rewriting them to settle-then-assert may legitimately turn them red.
|
||
|
Joseph Doherty
|
4dc2ad8084 |
feat(sql): implement the design §8.1 catalog identifier-validation gate (#496)
Until now ISqlDialect.QuoteIdentifier was the SOLE defence for authored
table/column names: an identifier went from the TagConfig blob straight into a
command text, bracket-quoted. The residual risk was bounded — a hostile name is
quoted into one nonexistent object, the query fails after the connection opens,
and the tag Bad-codes — but "bounded" is not "filtered", and the design promised
a filter. Both ISqlDialect and SqlServerDialect carried a doc paragraph saying so.
SqlCatalogGate + SqlCatalogLoader now resolve every authored identifier against
the live catalog at Initialize, and REPLACE it with the catalog's own spelling.
The identifier text in an emitted poll query is therefore a string this driver
read back out of ListSchemas/ListTables/ListColumns — not operator input. Quoting
becomes the backstop it was documented to be.
Decisions worth knowing before touching this:
- Substitution, not just validation. Matching is exact-ordinal first, then a
UNIQUE case-insensitive hit: SQL Server's default collation is CI so case
variants have always worked, and rejecting them would break valid deployments.
An ambiguous CI match under a case-sensitive collation is refused rather than
guessed — picking one would publish another column's data under the operator's
node, which is worse than rejecting the tag.
- Charset check BEFORE catalog lookup. Each identifier goes through
QuoteIdentifier for its rejection rules first, so a name carrying a control or
Unicode format character is rejected WITHOUT its value being echoed into a log
line (Trojan-Source). A name that passes is safe to render, which is why
catalog-miss messages do name it — an operator hunting a typo has to see what
they wrote. Both halves are pinned by tests.
- A rejected tag keeps its node. It is dropped from the POLLED table but stays in
the AUTHORED table, so it still materializes and reads BadNodeIdUnknown —
§8.1's specified outcome. My first wiring dropped it from both, which deleted
the node instead; an existing test (ReinitializeAsync_recoversFromFaulted)
caught it. A status code can only be published by a node that exists, and a
missing address-space entry is far harder to diagnose than a Bad quality.
- An unreadable catalog FAULTS Initialize; it does not reject every tag. That is
the absence of evidence about the tags, not evidence against them — rejecting
all of them would serve a confidently-empty address space and send the operator
hunting typos that do not exist. Zero visible schemas is treated the same way,
because that is exactly what a missing GRANT looks like. Faulting lands
DriverInstanceActor in Reconnecting with its retry timer running.
- Bounded load: one schema list, one default-schema scalar, then one ListTables
per distinct authored schema and one ListColumns per distinct authored relation
— never a full catalog enumeration, and nothing after Initialize. Every
authored name reaches the catalog queries as a bound @schema/@table parameter,
so building the allow-list cannot itself be an injection vector.
- ISqlDialect gains DefaultSchemaSql ("SELECT SCHEMA_NAME()"). An unqualified
`TagValues` must resolve in whatever schema the SERVER reports; hardcoding dbo
would be a silent lie on any estate that maps service accounts to their own
default schema. It is a query, not a constant, because the answer is
per-connection.
- Accepted v1 limitation: a 3-part db.schema.table (or linked-server name)
addresses a catalog this connection cannot enumerate, so it cannot be
allow-listed and is rejected with a message pointing at the fix — expose the
data through a view in the connected database.
VerifyLivenessAsync's wall-clock pattern is extracted to RunBoundedAsync and
reused, so the new I/O gets the same R2-01 / STAB-14 protection rather than a
second hand-rolled copy. (It also fixes a latent bug in the extracted code: the
OperationCanceledException arm detached the wrong task.)
Tests: 24 pure gate tests + 9 end-to-end driver tests against the real SQLite
catalog + 3 new live tests against the real SQL Server on 10.100.0.35 (21 in that
suite now pass, exercising the real SELECT SCHEMA_NAME() + INFORMATION_SCHEMA
path). Verified falsifiable: bypassing the gate turns exactly the three rejection
assertions red and leaves the rest green.
SqlInjectionRegressionTests is deliberately NOT rewritten to expect
BadNodeIdUnknown. It drives SqlPollReader directly, below the gate, and pins the
quoting backstop on its own — defence in depth is only worth the name if each
layer holds independently. Rewriting those assertions would delete the backstop's
only coverage and leave the gate a single point of failure. Its scope note, which
said the gate does not exist, is updated to say why it stays where it is.
|
||
|
Joseph Doherty
|
1919a8e538 |
feat(config): reject a persisted Sql connectionString at the deploy gate (#498)
The Sql driver's "a pasted literal connection string is never persisted" guarantee was
enforced only on the READ path: SqlDriverConfigDto has no connectionString property and
UnmappedMemberHandling.Skip drops the key on deserialization. Nothing stopped it being
WRITTEN. Config blobs are schemaless JSON columns and the fallback authoring pattern for
a driver without a typed form is a raw-JSON textarea, so an operator pasting
{"connectionString":"Server=...;Password=..."} would put a live credential in the
ConfigDb — persisted, replicated to every node's artifact cache, readable by anyone with
config access — while the runtime silently ignored it and the driver failed to connect.
Discarding a secret on read is not the same as refusing to store it.
DraftValidator.ValidateSqlConnectionStringNotPersisted fails the deploy
(SqlConnectionStringPersisted) when a Sql-typed driver instance's DriverConfig, or the
DeviceConfig of any device beneath it, carries a top-level connectionString key.
- Both surfaces are checked because DeviceConfig is merged onto DriverConfig before the
DTO sees it — a credential pasted into the device blob is the identical leak.
- The key is matched case-insensitively: System.Text.Json binds ConnectionString to a
connectionString property by default, so an ordinal match would leave the obvious
bypass open.
- Only the top level is scanned; the DTO is flat, so a nested occurrence cannot bind and
is not the mistake this rule exists to catch.
- The message never echoes the value — validation errors reach the AdminUI, the deploy
log and the audit trail, and repeating the string there would leak the credential the
rule is refusing to store. Pinned by a test.
- Malformed/blank/non-object JSON never throws: a parse failure would take down every
other rule in the same pass.
16 tests. Verified falsifiable — with the rule disabled the 6 positive assertions fail
and the 10 negatives stay green, so none of them passes vacuously.
Design §8.2's "if an inline connectionString is ever permitted (dev convenience only),
the AdminUI redacts it" carve-out is withdrawn in the same change: redaction only hides a
credential that has already been written.
Not covered, and filed as a follow-up: ClusterNode.DriverConfigOverridesJson is a third
persistence surface for a driver config blob, but it is not part of DraftSnapshot, so
this validator cannot see it.
|
||
|
Joseph Doherty
|
b95b99ba8e |
fix(drivers): correct 16 wrong OPC UA status-code constants + guard them by reflection (#497)
Every value verified against Opc.Ua.StatusCodes in the pinned SDK (1.5.378.106) by reflection, not by copying siblings. All are client-visible: OPC UA clients branch on status. The three named in #497: - Historian.Gateway SampleMapper "BadNoData" 0x800E0000 -> 0x809B0000 (0x800E0000 is BadServerHalted) - Galaxy "BadTimeout" 0x800B0000 -> 0x800A0000 (0x800B0000 is BadServiceUnsupported) - TwinCAT BadTypeMismatch 0x80730000 -> 0x80740000 (0x80730000 is BadWriteNotSupported) BadTypeMismatch was wrong in three MORE drivers the issue did not name — FOCAS, AbLegacy and AbCip carried the same 0x80730000. Every call site is a FormatException/InvalidCastException conversion failure, so the name was right and the value was wrong in all four. Adding the reflection guard then surfaced nine further defects offline tests had never checked: - Galaxy GoodLocalOverride 0x00D80000 -> 0x00960000 (not a UA code at all) - Galaxy UncertainLastUsableValue 0x40A40000 -> 0x40900000 - Galaxy UncertainSensorNotAccurate 0x408D0000 -> 0x40930000 - Galaxy UncertainEngineeringUnitsExceeded 0x408E0000 -> 0x40940000 - Galaxy UncertainSubNormal 0x408F0000 -> 0x40950000 - TwinCAT BadOutOfService 0x80BE0000 -> 0x808D0000 (was BadProtocolVersionUnsupported) - TwinCAT BadInvalidState 0x80350000 -> 0x80AF0000 (was BadAttributeIdInvalid) - AbCip/AbLegacy GoodMoreData 0x00A70000 -> 0x00A60000 (was GoodCommunicationEvent) - Historian.Gateway GatewayQualityMapper Good_LocalOverride 0x00D80000 -> 0x00960000 Galaxy's whole Uncertain block read as though the OPC DA quality byte could be shifted into the UA substatus position; it cannot — the substatuses are an unrelated enumeration. GatewayQualityMapper already held the correct table, which is what the Galaxy values are now reconciled against. Guard: StatusCodeParityTests (Core.Abstractions.Tests, which already project- references every driver) reflects over every `const uint` in the deployed ZB.MOM.WW.OtOpcUa.Driver.*.dll whose name reads as a status code, and asserts it equals Opc.Ua.StatusCodes.<name>. Discovery is by convention, so a new driver assembly referenced by that project is covered with no edit here. It went red on all nine defects above before the fixes and now checks 109 constants green. Because reflection can only see a NAMED constant, two inline literals were hoisted so the guard can reach them: Galaxy's BadTimeout/Bad into StatusCodeMap, and GatewayQualityMapper's 15-entry table into a new HistorianStatusCodes. An inline `0x800B0000u, // BadTimeout` at a call site is exactly how the Galaxy defect survived. The drivers stay SDK-free by design; only the test project references Opc.Ua.Core, as the oracle. Also corrected: tests that pinned the wrong values (Galaxy StatusCodeMapTests, SampleMapperTests, GatewayQualityMapperTests — all written from the same bad source), a mislabelled comment in DriverInstanceActorTests, and stale constants in two design docs, one of them the unexecuted MTConnect driver design that would have propagated BadNoData's wrong value into a new driver. The CLI's SnapshotFormatter value->name table was checked against the SDK and is correct; no change needed. |
||
|
Joseph Doherty
|
ee12568cab |
refactor(modbus-rtu): distinct CrcMismatch/UnitMismatch desync reasons
The RTU framer overloaded DesyncReason.TruncatedFrame for three distinct failures — a genuine short read, a CRC-16 validation failure, and a CRC-valid reply from the wrong RS-485 slave. Split the latter two into dedicated CrcMismatch / UnitMismatch enum members so a future .Reason consumer (metrics, operator diagnostics) can tell a wiring/EMI CRC fault apart from a mis-addressed multi-drop reply. Behaviour is unchanged — all three still throw ModbusTransportDesyncException and tear the socket down. Framing tests now assert the specific Reason on each path. Follow-up #12 from the Modbus RTU-over-TCP plan. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
b295b24419 |
feat(adminui): author + configure the Sql driver in /raw
The read-only Sql driver's runtime factory/probe and tag-side editors were wired, but the AdminUI could not author the DRIVER: the /raw "New driver" type list, the DriverConfigModal switch, and the legacy identity dropdown all omitted Sql, and no SqlDriverForm existed. Live-driving the AdminUI surfaced this. - Add SqlDriverForm.razor over the SqlDriverConfigDto shape: Provider (SqlServer-only in v1), required connectionStringRef (an env-var NAME, not a connection string — label is explicit), optional poll/operation/command timeouts, maxConcurrentGroups, nullIsBad. Enums serialize by NAME via JsonStringEnumConverter; a connection-string literal can never be authored (no such field is collected); rawTags (composer-owned) + allowWrites (inert, read-only v1) are never emitted. - Wire Sql into DriverConfigModal switch, RawDriverTypeDialog type list, and DriverIdentitySection legacy dropdown. - Add SqlDriverFormContractTests: the exact JSON the form emits constructs a driver through the real factory, missing connectionStringRef is rejected, and provider round-trips as the "SqlServer" name (never an ordinal). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
585f827ef3 |
fix(sql): close the residual credential leak in HandlePollError
The C1 credential-hygiene fix converted InitializeAsync and ReadAsync to
surface only ex.GetType().Name, but left HandlePollError building its
operator-facing Degrade() message from raw ex.Message, gated only by
'if (ex is DbException) return;'. That guard exists for the I1
double-classification concern, not for message safety, and it is incomplete
for the leak vector: a malformed connection string throws ArgumentException
from the keyword parser (the same unquoted-';'-in-password shape the sibling
SqlDriverBrowser.Sanitize special-cases), which is not a DbException and so
reaches Degrade(ex.Message) unredacted.
It is unreachable in today's control flow only because the connection string
is static and validated identically at Initialize first — an emergent
property, not an enforced invariant, and a live per-poll leak the moment a
future edit (refresh-on-reinit, a different provider) breaks that assumption.
Make the fallback type-only like the other two sites; the full exception still
reaches the log sink via the exception parameter. The I1 defer + control tests
are unaffected (they assert Degraded, not message text). Pinned by a new test
driving a credential-bearing ArgumentException through HandlePollError;
verified load-bearing (red against Degrade(ex.Message), green after).
Closes the C1 review finding on commit
|
||
|
Joseph Doherty
|
9b5a96876e |
feat(sql): register Sql factory + probe in Host, add DriverTypeNames.Sql
Add DriverTypeNames.Sql (+ All) as the shared source of truth, and wire the driver into the Host: register the factory in DriverFactoryBootstrap.Register via Driver.Sql.SqlDriverFactoryExtensions.Register(registry, loggerFactory), add the SqlProbe (= Driver.Sql.SqlDriverProbe) probe via TryAddEnumerable, and add the Driver.Sql project reference to the Host. Default tier A (SQL client is managed + cross-platform, so ShouldStub needs no change). Repoint the interim SqlDriver.DriverTypeName / SqlDriverFactoryExtensions.DriverTypeName literals at DriverTypeNames.Sql; both still resolve to "Sql", so the AdminUI TagConfigEditorMap/TagConfigValidator keys and the DriverTypeName-parity test stay green. The Core guard test discovers factories from its own bin, so it also gains a Driver.Sql project reference — that is the "registered-factory side" that lets DriverTypeNamesGuardTests' bidirectional-parity check pass (4/4 green). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
fcc7ed698e |
harden(modbus-rtu): factory throws on unknown transport + document RTU FC-shape assumption
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
898c7365c4 |
test(sql): close the blackhole gate's leaked-pause race + bound the shell command
Review follow-up on the blackhole gate. Two robustness gaps, both bounded to the disposable dedicated container (never shared infra), fixed: - If the outer token fired while 'docker pause' was in flight, 'paused' was set only AFTER the await, so a cancellation there left paused=false while the container could still complete the pause — the finally then skipped unpause and leaked a frozen container. Mark paused BEFORE the await, and make the finally's unpause best-effort (unpausing a never-actually-paused container errors harmlessly, and a cleanup failure must never mask the real test outcome). - The pause/unpause shell commands had no wall-clock bound of their own — only the post-pause read was capped — so a hung SSH handshake would hang CI. Give RunShellCommandAsync its own 30s hard cap that kills the process and throws TimeoutException, distinct from an operator's own cancellation. Offline skip still clean (1 skipped, 9ms, no socket/docker). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
37f444b99c |
fix(sql): stop leaking provider exception text; de-dup health classification; guard Faulted
C1: SqlDriver's Initialize/Read failure paths no longer interpolate the ADO.NET provider's ex.Message into LastError, the thrown message, or host status — only ex.GetType().Name reaches the operator surface; the full exception goes to the log sink via the structured logger's exception parameter. The driver is dialect-agnostic over an arbitrary DbProviderFactory, so it cannot rely on any provider's message discipline (Microsoft.Data.SqlClient's parser echoes an unrecognised keyword lower-cased, which a value-based redactor misses). C1a: replace the vacuous credential test (SQLite's "unable to open" message never contains the connection string, so it passed regardless) with one that fabricates a DbException whose own .Message carries a credential token and drives it through the Initialize liveness-failure path via the injectable factory seam — red against the pre-fix ex.Message interpolation. I1: HandlePollError now ignores the DbException class ReadAsync already classified, so a subscribed-read outage is classified (and logged) once, not twice with the worse message. I2: the poll's Healthy verdict routes through a shared SetHealthUnlessFaulted guard (the one Degrade already used), so a late in-flight poll cannot un-fault a driver a concurrent ReinitializeAsync just faulted. I3: DiscoverAsync warns when materializing an omitted-type tag as String, the only operator signal for the declared-vs-published type mismatch on a numeric column. M1: BuildTagTable wraps the per-entry parse so a stray non-JsonException throw skips the tag instead of stranding health at Initializing (it runs before Initialize's try/catch). M2 left as a documented TODO to avoid touching SqlPollReader. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
443b718158 |
feat(sql): typed AdminUI Sql tag-config model + validator (string enums)
SqlTagConfigModel + SqlRowSelectorModel mirror SqlEquipmentTagParser's accept/reject boundary byte-for-byte on the fields they own: model/type serialize as NAME strings (never numbers), KeyValue requires table+keyColumn+keyValue+valueColumn, WideRow requires table+columnName+a selector (where-pair OR topByTimestamp), Query is rejected, and unknown keys (top-level and nested rowSelector) survive load->save. Registered in TagConfigEditorMap + TagConfigValidator keyed off SqlDriver.DriverTypeName (DriverTypeNames.Sql is deferred to Task 11). A minimal placeholder SqlTagConfigEditor.razor lands the map entry now; Task 20 fleshes out the UI. The load-bearing test rounds editor output back through SqlEquipmentTagParser.TryParse (editor-output <=> parser-input agreement). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
02f2dafe2a |
test(sql): correct the cancelled-token test's overclaim
Its doc said it pinned "the real provider's cancellation path", but the token is cancelled before ReadAsync so the OCE is raised at the reader's SemaphoreSlim.WaitAsync gate, before any SqlConnection opens — it never exercises Microsoft.Data.SqlClient's in-flight command cancellation. Softened (option (a)) to state accurately what it proves: an already- cancelled token propagates as OCE and is never swallowed into a Bad snapshot or an unreachable-database verdict. The in-flight/deadline path is covered by SqlBlackholeTimeoutTests. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
10a6a37ca5 |
test(sql): blackhole/timeout live-gate on a dedicated mssql container
Adds the frozen-peer / BadTimeout gate for the Sql driver — the single highest-value integration test. Mirrors the S7 R2-01 blackhole gate: docker pause a DEDICATED mssql mid-poll, assert the next read surfaces BadTimeout within the client-side operationTimeout (≈3s) and well below the server-side CommandTimeout backstop (30s), the driver degrades (Degraded + host Stopped), and docker unpause recovers to Healthy/Running. - Docker/docker-compose.yml: disposable `otopcua-sql-blackhole` mssql on :14333 (never the shared :14330 ConfigDb server) + one-shot seed. - Docker/seed.sql: the two sample tables. - SqlBlackholeTimeoutTests.cs: env-gated (SQL_BLACKHOLE_ENDPOINT); pauses ONLY the hard-coded container name; skips loudly if the endpoint is the shared port 14330; bounds its own wait so a wedged impl fails, not hangs. Offline: clean skip (no socket, no docker shell-out). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
a55b7e51ca |
test(sql): injection regression — bind values, reject unknown identifiers
Locks the driver's injection guarantee against the SQLite fixture. An
authored VALUE ('; DROP TABLE TagValues; --) binds as a DbParameter and can
only ever be a key that matches no row (BadNoData); the seed table survives.
A hostile IDENTIFIER in table/column position is dialect-quoted into a
single nonexistent identifier — inert — and the payload never executes.
Scope, stated plainly: design §8.1 also specifies a catalog gate (validate
an authored identifier against INFORMATION_SCHEMA, reject an unknown one as
BadNodeIdUnknown). That gate does NOT exist in the driver yet and no task
here builds it, so a hostile identifier is not rejected up front — it is
quoted, the query fails after the connection opened, and the tag Bad-codes
as a query failure (BadCommunicationError), not BadNodeIdUnknown. These
tests assert what the code actually guarantees — the payload is inert and
the table intact — rather than a catalog gate that isn't there. The gate is
a tracked follow-up.
No implementation change: the reader already binds correctly (Task 7); this
suite pins it. Falsifiability-checked — forcing a real DROP before the
row-count assertion turns it red, so "table survives" is load-bearing.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
4c716242ac |
feat(sql): SqlDriverProbe SELECT-1 liveness check
The AdminUI "Test Connect" probe for the Sql driver: open a connection, run the dialect's LivenessSql (SELECT 1) under a linked-CTS deadline, return green with latency or red with a reason. Implements IDriverProbe; never throws (malformed JSON, missing connectionStringRef, unprovisioned connection string, a provider open failure, timeout, cancellation all become red results). Parses the SAME factory DTO with the SAME JsonStringEnumConverter as SqlDriverFactoryExtensions (R2-11 factory parity, mirroring ModbusDriverProbe) and resolves connectionStringRef the same way, so a config that Test-Connects is the config that Deploys. Credential hygiene: the resolved connection string never reaches the result message. A provider exception can embed the data source in its OWN message (the real SqlException shape), so the catch-all names the exception TYPE only, never ex.Message. The regression test's fake connection embeds the connection string in its thrown message specifically so surfacing ex.Message would leak it — verified load-bearing by breaking the guard and watching it go red. ForTest injects factory+dialect for the offline SQLite fixture path. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
3cc8069150 |
feat(sql): factory + connectionStringRef env resolution + string-enum guard
SqlDriverFactoryExtensions turns a deployed DriverConfig blob into a live SqlDriver, and SqlConnectionStringResolver resolves the authored connectionStringRef NAME from Sql__ConnectionStrings__<ref> so credentials never ride in a config blob (design §8.2). Three behaviours worth naming: - String-enum guard. JsonOptions carries JsonStringEnumConverter + camelCase, so `provider` parses whether authored as a name or an ordinal and always round-trips as the NAME — the systemic AdminUI defect where a page serialised an enum numerically against a string-typed DTO. - Config validation lands here because nothing below does it. operationTimeout must be STRICTLY greater than commandTimeout (design §8.3); the reader and the driver stay deliberately usable with the pair inverted so the frozen-database tests can prove the client-side bound fires. Non-positive timeouts / poll interval and maxConcurrentGroups < 1 are rejected too. - Credential hygiene. The resolved connection string reaches the provider and nothing else: messages name the ref, the environment variable, or SqlDriver.Endpoint's credential-free server/database rendering. Both a log-leak and an exception-leak test cover it. DTO changes: - Adds RawTags (List<RawTagEntry>), following the Modbus precedent — the deploy artifact delivers a driver's tags this way and the DTO had no way to receive them, so an authored Sql tag could never reach the driver. - Deletes SqlProbeDto + the `probe` key. It had no consumer: SqlDriver was specified without a background probe loop, and deliberately so — its IHostConnectivityProbe state is a by-product of the Initialize liveness check and of every poll, i.e. a statement about traffic that actually happened rather than a synthetic ping. On-demand connectivity is Task 10's SqlDriverProbe. Shipping a config key that silently does nothing is worse than not shipping it; UnmappedMemberHandling.Skip means a blob that still carries `probe` keeps parsing. allowWrites stays inert (SqlDriver implements no write capability) but an authored `true` now WARNS rather than passing silently — the flag cannot do harm, an operator who believes writes are enabled can. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
48cbc26c34 |
test(sql): env-gated central-SQL integration fixture + read round-trip
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
9e7fa5d11f |
feat(sql): SqlDriverBrowser transient-connection open with env-ref + literal
Opens one transient DbConnection from a form-supplied driver-config blob and hands ownership to SqlBrowseSession, which closes it on disposal. A database is named either by connectionStringRef — resolved in the AdminUI process from Sql__ConnectionStrings__<ref>, with an unresolvable ref naming the exact missing variable — or by a pasted, session-only literal. The literal wins and the ref is then not read; it cannot arrive from a persisted blob (SqlDriverConfigDto has no such property), so its presence proves an operator typed it now. Credential hygiene is the point of the type: no cached config field, nothing persisted, and no connection text in any log line or exception. Live-probed: Microsoft.Data.SqlClient and Microsoft.Data.Sqlite keep connection-string values out of SqlException/SqliteException, but their connection-string PARSER echoes an unrecognised keyword verbatim — and an unquoted ';' inside a password splits, so the tail of the password is reported as a keyword (Password=Sup3r;SecretTail => "Keyword not supported: 'secrettail;connect timeout'."). The parser's message is therefore never surfaced; every other failure additionally passes through a substring redactor that drops the inner exception when it fires. DriverType comes from SqlDriver.DriverTypeName, the driver's interim local constant — DriverTypeNames.Sql is added by the driver-factory task. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
e3e3f5fb04 |
fix(sql): report an ambiguous wide-row selector; pin the zombie-slot bound
Three review findings against SqlPollReader (
|
||
|
Joseph Doherty
|
861a1d1df0 |
feat(sql): SqlBrowseSession schema-walk over dialect catalog
Walks schemas -> tables/views -> columns via ISqlDialect's catalog SQL, with @schema/@table bound as parameters at every level. Column leaves carry the dialect-mapped DriverDataType in the attribute side-panel (ViewOnly, read-only v1). The NodeId encoding deliberately departs from the design sketch's literal `schema.table|column`: SQL Server permits `.` and `|` inside a quoted identifier, so that form mis-parses (main.a.b|c reads equally as schema `main`+table `a.b` and schema `main.a`+table `b`) and silently binds an operator's tag to the wrong column. SqlBrowseNodeId encodes `<kind>:<part>[|<part>...]` with `\`/`|` escaped and the kind prefix carrying the arity; it is public because the picker body decodes it back. The session owns the connection it is handed and closes it on dispose -- the registry-held session is the only lifetime hook, so a non-owning session would leak one pooled connection per reaped picker. Per-call work stays bounded by the AdminUI's existing 20s linked CTS (BrowserSessionService.PerCallTimeout); no second deadline is invented here. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
9b30bdeb7a |
feat(sql): SqlDriver shell wiring PollGroupEngine + probe + read-only discovery
SqlDriver implements IDriver / ITagDiscovery / IReadable / ISubscribable / IHostConnectivityProbe / IAsyncDisposable over the landed SqlPollReader, mirroring ModbusDriver. IWritable is deliberately absent: v1 is read-only structurally, and every discovered variable is SecurityClass=ViewOnly. The shell classifies poll outcomes because nothing below it does. The reader honours IReadable literally — it throws only when the database is unreachable and Bad-codes everything else — so a frozen database returns all-BadTimeout snapshots through a perfectly successful PollGroupEngine tick. Without ObservePollOutcome the driver would report Healthy while every value was Bad. Connection-class codes (BadTimeout / BadCommunicationError) degrade health and report the host Stopped; authoring-class codes (unresolvable RawPath, absent row, type mismatch) change nothing, so a tag typo never reports the database down. It deliberately does NOT synthesise an exception to earn engine backoff: the engine's exception path publishes nothing, which would cost clients the Bad quality the reader went out of its way to produce. Initialize builds the authored RawPath table first (pure, cannot fail; a malformed TagConfig is logged and skipped) then verifies liveness over one open-use-dispose connection bounded by wall clock as well as by token (the R2-01 lesson) — failure records Faulted and rethrows so DriverInstanceActor retries. The connection string is never logged: a credential-free server/database Endpoint is the only rendering that reaches a log, LastError, or the host status. DriverTypeNames.Sql is NOT added here — DriverTypeNamesGuardTests asserts bidirectional parity with registered factories, so the constant must land with the factory (Task 11). SqlDriver.DriverTypeName carries the string until then. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |