Commit Graph

798 Commits

Author SHA1 Message Date
Joseph Doherty 5dc8d34472 docs: the stale-Good quality contract, stated once (R3-02) 2026-08-20 10:56:16 -04:00
Joseph Doherty e3a72dc385 fix(runtime): virtual tags hold on Bad inputs and inherit the worst input status (archreview 02/S14) 2026-08-20 10:53:26 -04:00
Joseph Doherty 60e5b40228 fix(calculation): read cache carries real input status + timestamps 2026-08-20 10:40:15 -04:00
Joseph Doherty 6874a562f3 test+docs(mqtt): end-to-end Bad-then-identical-replay recovery 2026-08-20 09:55:11 -04:00
Joseph Doherty f679b2f10a docs(plans): annotate the r3 register — browse-transit-below-realm-root closed by 89630ee4+ff9d617a 2026-08-18 10:01:48 -04:00
Joseph Doherty 22ae18365b docs(acl): record the shared-root-folder browse-transit mapping
CLAUDE.md's "Browse needs ancestor visibility" bullet and docs/security.md's
ancestor-visibility bullet now name the topmost ancestor: the single shared
root folder (s=OtOpcUa, raw namespace) that both realm trees hang under, its
mapping onto the trie's zero-segment realm-root scope, the either-realm transit
(it fronts both trees), and the realm qualification.

Also corrects the adjacent security.md bullet that claimed "the two namespace
root folders remain visible" — there is one anchor, not one per namespace, and
what an ungranted session sees is the ns=0 Objects listing of it rather than a
browse into it. Flagged rather than silently left, since it contradicts the
mechanism the new bullet documents.
2026-08-18 09:45:09 -04:00
Joseph Doherty caf8830bbe docs(plan): r3 ledger finalized + Client.CLI logger-flake register entry 2026-08-18 08:50:11 -04:00
Joseph Doherty c4d6cba7e1 docs(plan): r3 close-out — outcomes, sweep record, register annotations 2026-08-18 08:48:40 -04:00
Joseph Doherty 326ca33800 docs(plan): record the T9 test rename in the live plan 2026-08-18 07:49:54 -04:00
Joseph Doherty 3e62f1617b fix(localdb): registration follows replication config — baseline on enable, deregister self-heal on disable (pair-symmetric)
LocalDbSetup.OnReady registered all four replicated tables unconditionally on
every driver node, replicating or not. That was not merely wasteful (an
unreplicated node paid two INSERTs plus a json_object of the whole row inside
every write transaction, for an oplog nothing ever drained) — it made
baselineExistingRows unusable, so enabling replication on a long-running
default-OFF node converged on SUBSEQUENT WRITES ONLY and the rows already in the
file stayed invisible to the peer, silently and forever.

Registration now follows one predicate, LocalDbSetup.ReplicationIsConfigured:
LocalDb:Replication:PeerAddress OR :ApiKey non-blank. The OR is load-bearing —
only the dialling half of a pair sets PeerAddress, so keying on it alone would
strip capture from every passive node and the pair would converge in one
direction only. SyncListenPort deliberately does not join it: the passive
endpoint is fail-closed on the key, so a port with no ApiKey syncs with nobody.

  configured   => RegisterReplicated(table, baselineExistingRows: true), per
                  table, in the unchanged load-bearing DDL->register order
  unconfigured => DeregisterReplicated(table), an idempotent boot self-heal that
                  drops the capture triggers an earlier build left behind and
                  prunes that table's oplog/row-version rows. One Information
                  line, only when it actually cleaned something.

PAIR SYMMETRY: the sync handshake compares registered-table digests fail-closed,
so flipping either key is a both-nodes-together change — set it on both and
restart both. Documented in CLAUDE.md, docs/Configuration.md and the replication
runbook (which claimed "this repo never calls DeregisterReplicated").

Verified against the pinned LocalDb 0.2.1: RegisterReplicated(string, bool) and
DeregisterReplicated(string) -> bool both exist; baselining seeds
__localdb_row_version at HLC 0 and the deregister return value distinguishes
"cleaned" from "nothing to clean". No pin bump needed.

Tests: LocalDbSetupTests gains pins for both branches (configured via
PeerAddress alone and via ApiKey alone; unconfigured => no tables, zero
triggers; late-enable => an HLC-0 baseline ledger entry; the log line fires once
and only when something was cleaned) — four went red against the old
unconditional code, and all four go red again under a constant-true predicate.
LocalDbWiringTests splits its exact-set pin into a replicating graph and a
default-OFF one. LocalDbPairHarness and AlarmSfLegacyMigratorTests built their
database from LocalDb:Path alone while configuring the replication engine from a
second config — production reads one configuration, so the two are now aligned.
Host.IntegrationTests: 224 passed, 0 failed, 5 skipped.
2026-08-18 07:25:01 -04:00
Joseph Doherty 088a1a1e81 docs(plan): r3 amendment — ScadaBridge register rows 38/39 join as Tasks 9-10 2026-08-18 07:00:42 -04:00
Joseph Doherty e9ba174156 docs(config): OTOPCUA_CONFIG_MODE is gone — site nodes are hardcoded FetchAndCache (Phase 4) 2026-08-17 07:07:51 -04:00
Joseph Doherty f64b10b964 docs(plan): perf follow-ups round 3 — r2 register closes + four local live gates 2026-08-17 07:06:07 -04:00
Joseph Doherty c15e5fc97b docs(plan): name the r2 close-out docs commit in the ledger 2026-08-17 06:11:36 -04:00
Joseph Doherty d4e5c379ce docs(plan): perf follow-ups r2 close-out — outcomes, sweep record, register annotations 2026-08-17 06:11:20 -04:00
Joseph Doherty 248d998a79 docs(plan): perf follow-ups round 2 — ACL rollover window, S7 coverage + TM/CT live pin, historian bounded watches, test-DB reaper, docker-dev readiness gate 2026-08-17 05:28:24 -04:00
Joseph Doherty 0b3972864a docs(plan): record the close-out docs commit SHA in the follow-ups ledger 2026-08-17 04:43:09 -04:00
Joseph Doherty 07fd17a34e docs(plan): perf follow-ups close-out — outcomes, live-gate evidence, register annotations 2026-08-17 04:42:51 -04:00
Joseph Doherty f06cbe16d9 docs(plan): perf follow-ups — 15-task plan off the remainder's follow-up register 2026-08-17 03:41:40 -04:00
Joseph Doherty cd010c7165 docs(plan): commit the execution ledger's final state 2026-08-16 05:41:34 -04:00
Joseph Doherty b9e9df4081 docs(code): honest rationales — Akka restart semantics, per-key shards, EF literal inlining; retire or annotate orphaned members 2026-08-16 05:33:33 -04:00
Joseph Doherty e75bb38734 docs: perf roadmap remainder — outcomes + follow-up register 2026-08-16 05:08:53 -04:00
Joseph Doherty 0298a0b944 chore(historian): batch-era defaults + appsettings/doc alignment 2026-08-16 04:48:55 -04:00
Joseph Doherty dc459633e0 docs(plan): perf roadmap remainder — historian batching, ACL cache, driver round-trips, alarm coalescing, AdminUI hot spots, residuals 2026-08-15 21:33:18 -04:00
Joseph Doherty a8c93a0080 docs: record surgical-deploys docker-dev live gate (PASSED) + correct rename-vs-identity rebuild wording
v2-ci / build (push) Successful in 5m5s
v2-ci / unit-tests (push) Failing after 17m48s
2026-08-15 20:01:37 -04:00
Joseph Doherty 0b537b5dec docs: record surgical-deploy semantics + artifact compression ops notes (plan 2026-08-15)
CLAUDE.md — a new "How a deploy reaches that address space" block under the v3
Batch 4 address-space section. The blanket ChangedRawTags/ChangedUnsReferenceVariables
=> Rebuild rule is gone, so the doc now names both halves explicitly: what still
rebuilds (alarm-bearing raw-tag deltas, raw-tag identity changes, raw container
renames/removals, changed equipment/alarms) and what is now surgical (non-alarm
attribute deltas, every UnsReferenceDelta). Also records the three semantics that
are easy to break by accident: mirror repair rides UnsReferenceVariable's inherited
backing fields (omitting a newly-inherited field silently reopens the staleness),
a degraded non-rebuild apply deliberately does not advance _lastApplied, and the
raw-realm announce gap is no longer neutral now that surgical adds keep
subscriptions alive.

CLAUDE.md ConfigSource — the RevisionHash == SHA-256(artifact bytes) paragraph is
still true and stays; it gains the clause that the blob is gzip-compressed since
2026-08-15 and the hash covers the COMPRESSED bytes, which is precisely why the
fetcher, cache and serve path were untouched.

docs/Uns.md + docs/Raw.md — one section each on what a deploy now tears down, from
the authoring surface's point of view, including the mirror repair semantics
(identity-preserving backing edits update in place and keep their MonitoredItems;
re-points and effective-name edits remove+recreate that one node).

docs/Configuration.md — "Artifact compression (2026-08-15)" under ConfigSource/
ConfigServe: gzip inside the hashed bytes, magic-sniffed on read, pre-upgrade
uncompressed rows still parse, plus the two operational facts an operator needs.
The first deploy after upgrade re-hashes every revision (same config, new bytes,
new RevisionHash) so it is a one-time full re-dispatch rather than a no-op; and
rollout is ordered — nodes must run artifact-sniffing code BEFORE central starts
emitting compressed blobs, since a pre-upgrade node handed a gzip artifact fails
the apply and keeps last-known-good until it is upgraded.

Plan doc — a "Follow-ups (recorded, not implemented)" section covering the seven
items surfaced during implementation and review: the raw-realm announce gap, the
RemoveEquipmentSubtree dangling-inverse-edge exposure, the missing end-to-end test
for a degraded remove-bearing apply's ratcheted rebuild, the scripted-alarm
thin-plan-ids-against-rich-rows blindness, the absent monotonic dispatch sequence,
LocalDb store ordering under concurrent StoreAsync, and threading the in-hand blob
through RestoreApplied.

tasks.json — all nine tasks completed.

Docs only; no source changes.
2026-08-15 18:02:21 -04:00
Joseph Doherty aec5ce565a docs: record value-pipeline batching (plan 2026-08-15)
- docs/v2/Galaxy.Performance.md gains a "Value-path batching (2026-08-15)"
  section: the batch seams (IBatchSubscribable / PollGroupEngine onBatch ->
  AttributeValueBatchPublished -> AttributeValueBatchUpdate -> sink WriteValues
  with 512-chunked single-lock batches in the node manager), the invariants (#519
  status codes verbatim, per-ITEM metering, Modbus deadband honored via the
  driver-filtered buffer, the ValueWriteBatchException partial-failure contract),
  the RealmKey re-key, and the pinned dispatcher's throughput 1 -> 100.

- The plan doc gains a "Follow-ups (recorded, not implemented)" section: the
  PollGroupEngine unconditional-collection trap (a future per-tag filter in
  S7/AbCip/FOCAS would silently reintroduce the deadband-bypass bug), a two-level
  driverId -> (rawPath -> refs) routing map, null/empty guard consistency on the
  internal batch messages, a possible DriverHostActorLiveValueTests merge, and the
  pre-existing aborted-cycle tail-loss on the three unbuffered drivers.

- tasks.json: all 13 tasks completed.

CLAUDE.md needed no edit: its only statement in this area ("One driver publish for
a RawPath fans (in DriverHostActor) to the raw NodeId AND every referencing UNS
NodeId") is still true — batching changed the message granularity, not the
fan-out semantics.

Verification: dotnet build ZB.MOM.WW.OtOpcUa.slnx clean (0 errors, no new warning
classes, zero warnings in any production file). 9 unit projects green, 3470
passed / 5 skipped / 0 failed: Core.Abstractions 305, Core 351+1s, Commons 348,
Runtime 575+3s, OpcUaServer 424+1s, Modbus 399, S7 302, AbCip 410, FOCAS 356.
No DB-backed suite was run — tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests does not
exist, and nothing this branch touches is covered by the SQL-backed projects.
2026-08-15 13:10:55 -04:00
Joseph Doherty 5c61d0b133 fix(modbus): exception-safe batch wrapper; driver batch wiring consistency 2026-08-15 12:27:19 -04:00
Joseph Doherty 477cb10ff2 docs(plans): value-pipeline batching + surgical deploys implementation plans 2026-08-15 12:04:56 -04:00
Joseph Doherty a1a258d85d chore(deps): bump ZB.MOM.WW.LocalDb 0.1.3 -> 0.2.1, correct the MaxBatchSize rationale
Closes the pin skew against ScadaBridge (which already pins 0.2.1). Pure pin
bump — no code change was required:

- `RegisterReplicated(string)` still delegates to `baselineExistingRows: false`,
  so `LocalDbSetup.OnReady` is byte-identical in behaviour.
- Everything 0.2.x added is additive (`DeregisterReplicated`, `MaxBatchBytes`,
  the `baselineExistingRows` overload); none of it is called here.
- The wire protocol is unchanged (0.1.x peers still sync) and the on-disk
  bookkeeping schema v1 -> v2 upgrades in place on open, so no rig, volume or
  operator action is needed.

The one thing that DID need reconciling is documentation. 0.2.0 made batching
byte-budgeted: `MaxBatchBytes` (2 MB default, under the 4 MB gRPC cap) now
bounds the wire message, and `MaxBatchSize` is demoted to a secondary row cap
plus the DB read-page size. The rig's `MaxBatchSize = 16` was chosen as a
hand-computed byte proxy ("row-count-only against gRPC's 4 MB cap"), and that
rationale is now wrong everywhere it appeared — appsettings, Configuration.md,
the pair-replication runbook, and both docker-dev site-a services.

The VALUE stays 16, for a different and still-real reason: OplogStore /
SnapshotStreamer materialise a whole read page in memory *before* the
byte-budget split runs, so 16 x ~171 KB artifact chunk rows bounds that
transient allocation at ~2.7 MB. Documented as such, with `MaxBatchBytes`
added to both config tables.

CLAUDE.md records the pin and two deliberate non-adoptions: this repo does not
take ScadaBridge's conditional-registration + boot-time `DeregisterReplicated`
self-heal (OnReady registers all four tables unconditionally, and
LocalDbSetupTests pins that set), and consequently does not use
`baselineExistingRows` either — so enabling replication on a long-running
default-OFF node converges on subsequent writes, not on rows already in the
file. Both are behaviour changes, out of scope for a pin bump.

Verification: solution builds clean; full suite run (57 projects). All 66
LocalDb tests in Host.IntegrationTests green. One unrelated failure,
RoslynVirtualTagEvaluatorTests.Evaluate_racing_ClearCompiledScripts_never_
fails_with_disposed — a Roslyn evaluator disposal race with no LocalDb in its
path, whose own comment documents that its "at most one clear per evaluation"
assumption depends on thread-pool pressure. It passed 3/3 re-runs in isolation
and fails only under full-solution parallel load; pre-existing test-design
fragility, not a regression from this bump.
2026-08-15 03:33:53 -04:00
Joseph Doherty c51c59987b fix(galaxy): publish an unrepresentable value as BadTypeMismatch, not a good null
v2-ci / build (push) Successful in 6m23s
v2-ci / unit-tests (push) Failing after 20m2s
Reverses the conservative call from the previous commit at the user's direction,
and they were right: "surface it on a counter first" left the wire lying. An
MXAccess event can carry Good quality while its payload is a kind MxValueDecoder
has no mapping for, so the old behaviour published null-under-Good — which a
client cannot tell apart from a genuine null reading. A value the gateway DID
send would be consumed as a value it did not, with the only evidence in a log
nobody is tailing. Bad data is the honest wire representation of lost data.

BadTypeMismatch is an existing StatusCodeMap constant, deliberately reused rather
than written inline: StatusCodeParityTests finds status constants by reflecting
over Good/Uncertain/Bad-prefixed uint fields and checking them against the pinned
SDK, so an inline literal would be invisible to that guard (Gitea #497, where 16
wrong constants had accumulated across 6 drivers exactly this way).

Logging is now once per DISTINCT value kind rather than once per pump. The set is
bounded by the MxValue oneof enum so a broken producer still cannot flood, but a
second unrepresentable kind appearing later gets its own line — warn-once-per-pump
would have silently swallowed it, which is the same class of bug as the null this
commit fixes.

The test's fixture matters more than its assertion: the undecodable event carries
Quality = 192 (OPC DA Good), so a decoder that gave up silently would publish Good
and fail. Paired with a positive control — a decodable value at the same quality
must still publish Good — so it cannot be satisfied by a pump that marks
everything Bad. Falsified by reverting the status to Good: the test reds, and only
that test.

Verified: 331 Galaxy tests green, Core.Abstractions 298 (the parity guard),
solution builds 0 errors. Docs corrected in the two places that recorded the old
decision as deliberate.
2026-08-12 03:03:51 -04:00
Joseph Doherty 4769044f88 feat(galaxy): handle the replay-gap sentinel and sparse-array values
v2-ci / build (push) Successful in 4m39s
v2-ci / unit-tests (push) Failing after 19m35s
Closes the two follow-ups left open by the MxGateway 0.2.0 bump. Reading the
gateway's own contract first changed what each of them is, and one of my earlier
claims was simply wrong -- both corrections are recorded below.

Replay gap
----------
A sentinel MxEvent carries family = UNSPECIFIED, so it fell through EventPump's
family switch into the default drop: we silently ignored a documented MUST ("you
missed events -- discard local state and re-snapshot"). It is now intercepted and
answered by re-issuing SubscribeBulk for every tracked subscription, since
MXAccess delivers an initial value per item on subscribe -- re-subscribing IS the
re-snapshot. It does not restart the event pump (the stream carrying the sentinel
is healthy, and the handler runs on that stream's own read loop, so disposing the
pump from there would deadlock on itself), it does not block the read loop, and
re-entry is gated so a burst collapses to one pass instead of a subscribe storm
per sentinel.

Handled on the READ loop, not in the dispatch loop, because everything past that
point is subject to newest-dropped channel pressure -- and "you missed events" is
the one signal whose loss is unrecoverable. Pinned by a test that fills the
channel and still expects the sentinel.

⚠️ This cannot fire in the shipped configuration, and I did not pretend otherwise
or paper over it by enabling resume. The gateway emits the sentinel only for a
stream opened with after_worker_sequence > 0; GatewayGalaxySubscriber passes 0,
which the gateway treats as "fresh stream, not a resume -- plain attach, no
replay, no sentinel". Enabling resume is a larger and genuinely delicate change:
the sequence is only meaningful within ONE gateway session, our reconnect
recreates the session, and replaying a remembered sequence against a fresh
session sets the server's live watermark ABOVE every event it is about to send --
silencing the stream instead of recovering it. That deserves its own change with
a live gate, which I cannot run from here.

Sparse arrays -- correcting the last commit message
---------------------------------------------------
I wrote that the bump "makes handling it possible" on the decode path. That is
wrong: MxSparseArray is documented write-only, the gateway expands it before
forwarding, and "the worker never receives or produces it". So the decode path
is unreachable by contract and the write path has no use for it either -- it is a
wire compaction for a WHOLE-array write, not a partial update (unmentioned
indices are RESET, deliberately, because the gateway cannot read-modify-write
without racing the provider). It is therefore the wrong primitive for an OPC UA
IndexRange write, which expects untouched elements preserved -- and WriteRequest
carries no IndexRange in the first place.

It is decoded anyway, as contract defence: the alternative if a future gateway
ever emits one is a silent null published under Good quality. Fill semantics
mirror the gateway's own SparseArrayExpander exactly (epoch for Time, empty
string, Integer widening to long[] when any element is int64) so a value that
round-trips through a write reads back as the same CLR array.

Also: the decoder is now total by contract, since it runs inside the dispatch
loop where an escaping exception kills dispatch for the life of the pump. A
malformed payload degrades instead of throwing.

A real defect found in the same code
------------------------------------
A stream the GATEWAY completes -- a closed or evicted session -- ends the RPC
without an error, so nothing threw, the supervisor was never notified, and the
pump exited silently. GetHealth() kept reporting Healthy, which also means the
#527 comms-loss watchdog could never trip, because it requires a non-Healthy
state. The driver went permanently deaf with no signal anywhere. This one is
live, unlike the two above. Now reported as a fault; the falsification control is
the existing CleanShutdown_DoesNotInvokeOnStreamFault, since our own disposal
also ends the stream and must NOT be reported.

Value kinds we cannot represent are now counted and warned once rather than
published as an indistinguishable null. The status is deliberately NOT downgraded
to Bad: nothing can reach that branch today, and changing what an OPC UA client
sees for a live shape is not a change to make without a gate. The counter exists
so the next person has evidence instead of a guess.

Verified: 330 Galaxy tests green (12 new), plus Galaxy.Browser 21, Runtime 558,
Core 351; full solution builds 0 errors with the warning census unchanged. Every
new test was falsified first -- disabling the gap interception reds 3 of 4 (the
throwing-callback test passes vacuously when the callback never runs, by
construction), disabling sparse decode plus the representability signal reds all
7 decoder tests, and both were restored and re-verified green.

Docs: the Galaxy.Performance invariant was stated as received = dispatched +
dropped + in-flight, which this breaks -- a sentinel is received but never enters
the channel -- so it now reads + replay_gaps. Galaxy.md gains the three
operational notes; DriverLifecycle.md records that "reaches a non-Healthy state"
is an obligation a driver can silently fail to meet, which is the general shape
of the clean-end bug.
2026-08-12 02:56:17 -04:00
Joseph Doherty 9e337efa0a docs(plan): AdminUI density sweep — §1d scan findings + 3-task execution plan 2026-08-10 17:56:27 -04:00
Joseph Doherty 8f8d8b300c merge: AdminUI sweep foundation — scoped-bundle link, btn sizing vars, notice-alert, tree-twist, doc relocations 2026-08-10 16:33:24 -04:00
Joseph Doherty 697a773340 fix(adminui): link the dead scoped-CSS bundle; add btn/notice-alert/tree-twist rules
Foundation task of the AdminUI UI sweep. CSS/link/doc only — no behavior change
beyond restoring styles that were built but never served.

- App.razor: link the RCL's scoped-CSS bundle
  (_content/ZB.MOM.WW.OtOpcUa.AdminUI/ZB.MOM.WW.OtOpcUa.AdminUI.bundle.scp.css).
  It is produced on every compile and served by the Host, but nothing referenced
  it, so ContextMenu.razor.css was dead at runtime and context menus rendered
  in-flow with no position:fixed.
- site.css: three additions.
  * .btn / .btn-sm sizing as Bootstrap CSS-VARIABLE overrides (never box
    properties — .btn-group depends on Bootstrap's own .btn rules for its seams
    and end-cap radii). Theme 0.3.1 ships no .btn rule, so Bootstrap's 1rem
    applied against the theme's 0.9rem body. Upstreaming candidate, same as the
    ignitionoee router's app.css.
  * .notice-alert — red variant of the kit's amber .notice, on the --bad/
    --bad-bg/--bad-border tokens. The 13 existing call sites hack this with
    style="border-color:var(--alert)", and no --alert token exists anywhere, so
    the tint silently no-ops today. Call-site migration is a separate task.
  * .tree-twist — shared 18px expander column for the tree components, replacing
    repeated inline style="width:18px". Rows are d-flex, so flex-basis is what
    actually holds the column. Adoption is a separate task.
- ContextMenu.razor.css: correct the header comment — the kit is light-only
  (no dark block in Theme 0.3.1, site.css, or this file). No rule changes.
- docs/Telemetry.md: record that driver-health is keyed per driver instance
  across the cluster, not per Akka member (the wire record carries no per-member
  identity). The fact previously lived only in Hosts.razor page copy.
- docs/Configuration.md: new audit-log subsection recording that EventId gives
  cross-restart dedup, enforced by the filtered unique index
  UX_ConfigAuditLog_EventId, so a retried apply inserts no duplicate row;
  CorrelationId joins a row to its originating workflow. Previously only in the
  historical v2 rebuild plan and ClusterAudit page copy.
2026-08-10 16:31:42 -04:00
Joseph Doherty 0355e83b31 docs(plans): AdminUI UI sweeps — discovery evidence + 6-task execution plan 2026-08-10 16:26:56 -04:00
Joseph Doherty b8acabfa5f merge: SQL driver health review findings resolved (Driver.Sql-004/-005/-006/-008/-010)
# Conflicts:
#	docs/drivers/Sql.md
2026-08-10 08:11:40 -04:00
Joseph Doherty ead8a97511 merge: MTConnect review findings resolved (Driver.MTConnect-004 write-back authoring, -005/-006/-007)
# Conflicts:
#	code-reviews/Driver.MTConnect/findings.md
2026-08-10 08:11:03 -04:00
Joseph Doherty 38dee82e29 merge: SQL contracts review findings resolved (Driver.Sql.Contracts-001 scalar keyValue, -002 fleet-wide numeric-enum rejection) 2026-08-10 08:10:36 -04:00
Joseph Doherty 298ad04fba feat(adminui): author MTConnect write-back on the driver form (Driver.MTConnect-004)
IWritable shipped behind write.enabled and docs/drivers/MTConnect.md told the operator to
set it, but MTConnectDriverForm — the only editor DriverFormMaps maps for this type, with
no raw-JSON escape hatch and no driver-instance API — exposed no write section, so the
documented enablement path was a hand edit of DriverInstance.DriverConfig in the config DB.

- Write-back section: switch + form text stating it is observation injection, that the
  Agent's AllowPut is an independent second gate, and that it does not by itself make a
  node writable over OPC UA (Tag.AccessLevel still composes CurrentWrite).
- Write added to the mirrored MTConnectConfigDto and to the form model, written in BOTH
  states — an omitted block would leave a previously-authored true standing, so the switch
  could never turn writing off again.
- The false 'MTConnect is read-only' header comment is gone; the doc names the new section.

No bUnit here, so the model/serialization seam is unit-tested both sides: the AdminUI test
pins the emitted bytes, and MTConnectFactoryTests parses that same document and asserts
MTConnectDriverOptions.Write.Enabled. Live page verification remains a rig step.
2026-08-10 08:07:36 -04:00
Joseph Doherty 678aa3aa53 fix(sql): read keyValue with the same scalar read as whereValue (Driver.Sql.Contracts-001)
The two value-bearing fields were read asymmetrically and nothing said why.
whereValue went through ReadScalar (string/number/boolean — the design doc's own
wide-row example authors it as the number 7); keyValue went through the string-only
ReadString, so a numerically-authored key refused the tag outright. Nothing downstream
justified the split: both are BOUND as DbParameters and both are matched back as text —
SqlPollReader.IndexByKey stringifies the key cell precisely so an authored key can face
an INTEGER key column.

The refusal was also mis-explained. Inspect branched on ReadString(root, "keyValue") is
null, so a numerically-authored key drew "Sql KeyValue tag has no 'keyValue'" — telling
the operator the field is missing when it is right there in the blob, the #549 shape of
an authoring surface disagreeing with the runtime about WHY.

keyValue now uses ReadScalar; Inspect mirrors it with HasScalar and says what a usable
key is; SqlTagConfigModel loads it with the shared TagConfigJson.GetScalarText (which
also replaces SqlRowSelectorModel's private copy), so the editor is not left stricter
than the parser. An object/array/null key is still refused. docs/drivers/Sql.md updated.

RED first: 4 failing in Driver.Sql.Tests.
2026-08-10 08:07:15 -04:00
Joseph Doherty 9d4760fd2f docs(driver.sql): topByTimestamp is only defined when the ordering column is unique (Driver.Sql-008) 2026-08-10 08:06:27 -04:00
Joseph Doherty 300abde4bb merge: SQL browser/probe review findings resolved (Driver.Sql-009, Driver.Sql.Browser-001/-002/-003) 2026-08-10 08:06:02 -04:00
Joseph Doherty 3427370831 docs(reviews): close Driver.Sql-009 + Driver.Sql.Browser-001/002/003
-003 is a documented Won't Fix: the mitigating control is the ConfigEditor page
policy on the only two surfaces that reach a browse session (/raw and /uns),
there is no HTTP endpoint for browse, and PageAuthorizationGuardTests pins both
pages to that exact policy. Recorded in docs/security.md under Control-Plane
Authorization and pointed at from SqlDriverBrowser's class summary.
2026-08-10 08:05:12 -04:00
Joseph Doherty a8cfc886a3 fix(sql-browser): rank the row-selector candidate so a date-only column is a last resort (Driver.Sql.Browser-001)
PreferredTimestampColumn nominated the first column mapping to DriverDataType
.DateTime, and that set includes date-only types (SQL Server / PostgreSQL
'date'). A table whose first date/time column is a 'date' committed
rowSelector.topByTimestamp = <date>, making the wide-row read ORDER BY <date>
DESC LIMIT 1 - which ties for every row written that day, so 'the newest row'
became storage order among the ties.

Time-bearing candidates now outrank date-only ones; a date-only column is still
stated when it is all the table has, so such a table stays bindable. Oracle's
DATE carries a time of day and is exempt - pinned by a control dialect that
differs from the SQLite one only in the Provider it reports.
2026-08-10 07:59:35 -04:00
Joseph Doherty 1c67d019e5 docs(mtconnect): describe the fixed browse-commit coercion type (#560)
The comments and docs that described Driver.MTConnect-003 as live at HEAD would
have been wrong the moment 64be41d8 landed:

- code-reviews/Driver.MTConnect/findings.md — Driver.MTConnect-003 Resolved,
  with the fixing commit; module open-finding count 5 -> 4.
- code-reviews/Driver.MTConnect.Contracts/findings.md — the one clause inside
  Contracts-001's prose that asserted 003 was Open.
- docs/drivers/MTConnect.md — the coercion-precedence warning now states that a
  browse-committed tag resolves at step 1 CARRYING the step-3 answer, and that
  step 3 remains the rescue for a hand-authored / CSV / API blob with no type.
- MTConnectDriver.BuildBinding, MTConnectDataTypeInference and
  MTConnectDriverOptions.RawTags remarks — same correction, each keeping a short
  historical note of what the defect was so a reader meeting the old blobs still
  understands them.

Comment-only in src; the driver suite (538) and AdminUI suite (1474) re-run green.
2026-08-10 07:36:45 -04:00
Joseph Doherty d68009106b merge: Mqtt runtime + Transport code review — deaf-reconnect subscription reset (High), teardown-before-reinit session leak (High), timeout normalization, dead CA-pin warning; 12 findings 2026-08-10 07:02:15 -04:00
Joseph Doherty 978eeda753 merge: MTConnect family code review — sample-pump hot-loop backoff, probe deviceName path, stale-premise docs; 9 findings (High browse-commit String coercion recorded cross-module) 2026-08-10 07:00:09 -04:00
Joseph Doherty 2b4b3fcbbf fix(mqtt): session-rebind subscriptions, re-init teardown, timeout clamp
Findings from the per-module review of Driver.Mqtt at b990adae
(code-reviews/Driver.Mqtt/findings.md records all ten; these are the fixed
ones).

Driver.Mqtt-001 (High) — MqttSubscriptionManager._subscribedTopics is a claim
about the broker session it established filters on, and AttachTo never cleared
it, but the manager OUTLIVES a session rebuild (AdoptOptions keeps it whenever
the ingest identity is unchanged). After a driver-host reconnect
(ShutdownAsync + InitializeAsync, the #527 watchdog's retry) or a
ReinitializeAsync that only changed the endpoint/credentials, the host's
re-subscribe resolved an empty pending set, sent no SUBSCRIBE at all, and left
the driver Connected/Healthy on a session carrying ZERO subscriptions — no
error, no Bad status, values frozen until the new session happened to drop.
AttachTo now forgets them, mirroring SparkplugIngestor.AttachTo's reset.

Driver.Mqtt-002 (High) — InitializeCoreAsync published _connection and
_primaryHost over whatever the driver already held, so a second initialize
stranded a connected, self-supervising broker session (plus a second Sparkplug
primary-host claim) that no later dispose could reach. Reachable without any
teardown of ours via DriverInstanceActor.AdoptConfigDuringInit. Now tears down
first, under the gate both callers already hold.

Driver.Mqtt-003 (Medium) — connectTimeoutSeconds is [Range]-annotated but
unvalidated on the runtime path; 0 makes every deadline fire immediately and a
negative value throws ArgumentOutOfRangeException out of ShutdownAsync and
DisposeAsync. Normalized once, at the seam every options instance passes.

Also: ShutdownAsync keeps the live last-message instant (-004), three usings
left behind by #512 removed (-005), stale remarks corrected (-009, -010), and
docs/drivers/Mqtt.md now distinguishes the two kinds of reconnect.

Tests 708 -> 713. All three regression tests were confirmed to fail against
the un-fixed code, with two controls that pass in either state.
2026-08-10 06:59:18 -04:00
Joseph Doherty 69ad376f0d review(Driver.Sql): findings + fix the missing default-schema fallback, the rowSelector doc, and a dead package ref
Driver.Sql-001 (Medium): SqlCatalogLoader.ReadDefaultSchemaAsync documented a
fallback to the sole visible schema when the server reports no default schema and
did not implement it, so a login with no DEFAULT_SCHEMA (or a Postgres search_path
that resolves to nothing) rejected EVERY unqualified authored table at the catalog
gate — a whole-driver BadNodeIdUnknown outage against a healthy database. Fixed;
SqlCatalogDefaultSchemaTests written red first, with a control proving the
ambiguous multi-schema case still refuses to guess.

Driver.Sql-003 (Medium): docs/drivers/Sql.md spelled the WideRow row selector at
the top level, but the parser reads those keys only inside a nested rowSelector —
a tag authored from the doc is refused and reads BadNodeIdUnknown (the #552 class,
via the documentation). Also removed the documented-but-nonexistent pollIntervalMs
per-tag field.

Driver.Sql-007 (Low): dropped the unused Microsoft.Data.SqlClient PackageReference
left behind by the .Dialect extraction (#526).

Open: 002 (no IN-list chunking vs the 2100/1000 provider bind caps), 004, 005, 006,
008, 009, 010.
2026-08-10 06:58:08 -04:00