Commit Graph

886 Commits

Author SHA1 Message Date
Joseph Doherty
a9be80923c docs(v2-release-readiness): drop stale 'this branch task-galaxy-e2e' ref
The task-galaxy-e2e branch was merged + deleted; the durable reference
is PR #205 alone. Tidies a dangling pointer that future readers might
chase looking for a branch that no longer exists.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 16:50:06 -04:00
Joseph Doherty
994997ba7b fix(driver-galaxy): vendor MxGateway.Client + MxGateway.Contracts as binary refs
The sibling mxaccessgw repo restructured: clients/dotnet/MxGateway.Client
no longer exists, and the proto contracts moved to a new namespace
(ZB.MOM.WW.MxGateway.Contracts.Proto, was MxGateway.Contracts.Proto). The
driver's source still expects the pre-restructure namespace, so the
broken ProjectReference produced 86 build errors in src/ + 1 in tests/
on master.

Resolution: vendor the last known-good build of MxGateway.Client.dll
(99 KB, May 22) and MxGateway.Contracts.dll (490 KB, May 23) under
src/Drivers/.../Driver.Galaxy/libs/, reference them via <Reference
HintPath=...> in both the driver and its test csproj, and declare the
NuGet packages the dropped ProjectReference was supplying transitively
(Google.Protobuf, Grpc.Core.Api, Grpc.Net.Client,
Microsoft.Extensions.Logging.Abstractions, Polly) at versions matching
the sibling repo's ZB.MOM.WW.MxGateway.Contracts.csproj so binary
compatibility is preserved.

Why this over a source migration:
  Source migration would require namespace renames across ~19 driver
  files PLUS reimplementing MxGatewayClient / MxGatewaySession /
  GalaxyRepositoryClient (~2,200 LoC) — the sibling repo dropped the
  client library entirely, keeping only the proto contracts. Vendoring
  the last known-good binaries unblocks the build in minutes, freezes
  the gateway contract surface at a known-good version, and preserves
  the option to migrate properly once the sibling repo decides whether
  to restore a client library or hand the work back to us.

libs/README.md documents the unwinding plan (either path closes the
debt: sibling restores a client library, or driver migrates to the new
contracts namespace + reimplements the client wrapper).

Verification:
  - dotnet build ZB.MOM.WW.OtOpcUa.slnx: 0 errors (was 87).
  - Driver.Galaxy unit tests: 245/245 pass.
  - Integration tests not run here (require a live mxaccessgw gateway).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 16:32:56 -04:00
Joseph Doherty
0001cdd579 fix(scripted-alarms): reuse per-alarm evaluation scratch on the hot path
Core.ScriptedAlarms-009 resolution: replace the per-call Dictionary +
AlarmPredicateContext allocation with a per-alarm reusable AlarmScratch
held in _scratchByAlarmId, refilled in place under _evalGate on each
evaluation. The hot path no longer allocates per upstream tag change.

Why this matters:
  On a busy line where many tags feeding many alarms change frequently,
  the old BuildReadCache allocated a fresh dictionary + context on every
  predicate evaluation — a steady stream of short-lived allocations the
  GC eventually has to reclaim. With the reuse, the dictionary and
  context are allocated once per alarm (on first evaluation) and refilled
  in place across every subsequent re-eval.

Implementation:
  - New private AlarmScratch class holds the reusable
    Dictionary<string, DataValueSnapshot> read cache (pre-sized to the
    alarm's Inputs.Count) and the AlarmPredicateContext that wraps it by
    reference. The context observes refilled values without being
    re-created.
  - ConcurrentDictionary<string, AlarmScratch> _scratchByAlarmId on the
    engine, cleared in LoadAsync alongside _alarms so a config-publish
    drops the prior generation's scratch (Inputs / Logger may change).
  - EvaluatePredicateToStateAsync looks up scratch via GetOrAdd, calls
    the new RefillReadCache(Dictionary, IReadOnlySet) helper to clear +
    repopulate the dictionary in place, then runs the predicate against
    the reused context.
  - BuildReadCache removed.

Safety:
  Reuse is serialised under _evalGate which guarantees no two threads
  ever observe the same scratch in a half-refilled state. The
  AlarmPredicateContext is bound to the scratch dictionary by reference,
  so the predicate's ctx.GetTag(path) sees the freshly-refilled values
  rather than a stale snapshot.

Verification:
  - All 66 ScriptedAlarms tests pass (was 63 — three new regression tests
    locking the reuse contract).
  - All 56 VirtualTags tests still pass (unchanged).
  - All 104 Core.Scripting tests still pass (unchanged).

New tests in ScriptedAlarmEngineTests:
  - Reevaluation_reuses_the_same_read_cache_dictionary — asserts
    ReferenceEquals(scratch_before, scratch_after) across two
    evaluations of the same alarm.
  - Reevaluation_reuses_the_same_predicate_context — same, for the
    context.
  - LoadAsync_drops_the_prior_generations_scratch — asserts a config
    publish wipes the prior scratch (so a stale Logger / Inputs can't
    leak into the new generation).

Internal test hooks TryGetScratchReadCacheForTest /
TryGetScratchContextForTest added via the existing
InternalsVisibleTo for the tests project. Kept internal — not part of
the public engine surface.

Docs:
  - docs/v2/Galaxy.Performance.md "Scripted-alarm engine" section
    rewritten as "hot-path allocation reuse" documenting the new
    contract + reuse safety reasoning + the three regression tests.
  - code-reviews/Core.ScriptedAlarms/findings.md -009 flipped
    Won't Fix → Resolved.
  - code-reviews/README.md regenerated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 16:10:09 -04:00
Joseph Doherty
7b6ab2ec6f fix(scripting): unload compiled-script assemblies via collectible ALC
Core.Scripting-008 resolution: replace the legacy CSharpScript.CreateDelegate
path with hand-rolled CSharpCompilation + Emit + collectible AssemblyLoadContext,
so per-publish compile accretion no longer requires a server restart to reclaim.

Why this was needed:
  Roslyn's CSharpScript path emits dynamically-compiled script assemblies into
  the default AssemblyLoadContext, which is non-collectible. Across config-
  publish generations each Clear() drops dictionary entries but the emitted
  assemblies stay loaded for process lifetime, so memory grows steadily on
  long-running servers with frequent publishes. The accepted-limitation note
  in docs/VirtualTags.md recommended scheduled restarts as the workaround;
  operator feedback was that restarts are difficult, so the underlying
  limitation was the right thing to fix.

Implementation:
  - New ScriptAssemblyLoadContext(name, isCollectible: true) hosts one emitted
    script assembly per evaluator.
  - ScriptEvaluator.Compile synthesises a wrapper class around the user source
    (CompiledScript.Run(globals) — explicit return required per ordinary C#
    semantics, which every existing script already uses), builds a
    CSharpCompilation against the sandbox references, runs the
    ForbiddenTypeAnalyzer over the semantic model unchanged, emits to an
    in-memory PE stream, loads via ScriptAssemblyLoadContext.LoadFromStream,
    and binds a strongly-typed Func<ScriptGlobals<TContext>, TResult> delegate
    via reflection.
  - ScriptEvaluator now implements IDisposable — Dispose calls
    AssemblyLoadContext.Unload(), which makes the emitted assembly eligible
    for GC at the next collection cycle.
  - CompiledScriptCache.Clear() disposes every materialised evaluator before
    dropping its dictionary entry; CompiledScriptCache itself is now
    IDisposable for graceful server shutdown.
  - ScriptSandbox.Build returns a new SandboxConfig (References + Imports)
    instead of a Roslyn ScriptOptions; references now span BCL via the
    TRUSTED_PLATFORM_ASSEMBLIES set filtered to System.* + netstandard +
    Microsoft.Win32.Registry, so forbidden BCL types resolve at compile and
    ForbiddenTypeAnalyzer is the sole security gate (consistent with the
    Core.Scripting-001 / -002 model — references-list-only restriction is
    porous against type forwarding, so the analyzer must be the real gate).

Verification:
  - All 104 Core.Scripting tests pass (was 101 — three new regression tests
    locking the unload contract).
  - All 56 VirtualTags tests pass (unchanged).
  - All 63 ScriptedAlarms tests pass (unchanged).
  - New CompiledScriptCacheTests:
    - Dispose_unloads_compiled_script_assembly_load_context — proves single-
      evaluator ALC unload via WeakReference + bounded GC.Collect() loop.
    - Clear_disposes_every_materialised_evaluator — proves publish-replace
      releases every prior generation's ALC.
    - GetOrCompile_after_Dispose_throws_ObjectDisposedException — locks the
      post-dispose contract.

Docs:
  - docs/VirtualTags.md "Compile cache" section rewritten: the accepted-
    limitation note replaced with the unload contract + the new authoring
    convention (explicit return).
  - docs/ScriptedAlarms.md cross-reference updated to drop the obsolete
    restart guidance.
  - code-reviews/Core.Scripting/findings.md Core.Scripting-008 flipped
    Won't Fix → Resolved with the implementation summary.
  - code-reviews/README.md regenerated.

Pre-existing breakage note: Driver.Galaxy fails the solution-wide build on
master because its ProjectReference to the sibling mxaccessgw repo's
MxGateway.Client targets a path that the sibling repo no longer has after a
recent restructuring. This is unrelated to Core.Scripting-008 and was
verified to exist on master before this branch was cut.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 15:55:04 -04:00
Joseph Doherty
5a9c4591b9 fix(cli-common): name native-driver-emitted status codes in SnapshotFormatter
Driver.FOCAS.Cli-005 follow-up: extend the SnapshotFormatter.FormatStatus
shortlist with the five Bad* codes the native-protocol mappers (FOCAS,
AbCip, AbLegacy) emit but which the shortlist previously left unnamed,
so they rendered only as severity-class 'Bad' instead of the documented
'BadDeviceFailure' / 'BadNotWritable' / ... names operators are told to
read off probe/write output.

Added entries:
  0x80020000 BadInternalError
  0x803B0000 BadNotWritable
  0x803C0000 BadOutOfRange
  0x803D0000 BadNotSupported
  0x80550000 BadDeviceFailure

(BadTimeout 0x800A0000 was already added under Driver.Cli.Common-001.)

Tests: SnapshotFormatterTests gains a new [Theory]
FormatStatus_names_native_driver_emitted_codes covering the five names,
and the existing well-known [Theory] is extended with the same entries
to enforce exact '0x... (Name)' rendering. Suite now 47 green (was 42).

Flips Driver.FOCAS.Cli-005 from Deferred to Resolved; README regenerated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 15:14:08 -04:00
Joseph Doherty
0f8ce1cb80 docs(code-reviews): regenerate index — final batch — 6 Low findings resolved
Batch 7 closed the last Open findings in Client.UI. The review backlog
is now empty: 0 Open findings across all 31 modules.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 11:25:28 -04:00
Joseph Doherty
1b10194634 fix(client-ui): resolve Low code-review findings (Client.UI-003,004,006,009,010,011)
- Client.UI-003: wire Serilog properly per CLAUDE.md — console sink +
  rolling daily file sink in Program.Main, Log.CloseAndFlush in finally,
  per-VM Log.ForContext<> loggers.
- Client.UI-004: migrate the cert-store folder picker from the obsolete
  OpenFolderDialog to StorageProvider.OpenFolderPickerAsync (with
  TryGetFolderFromPathAsync seed + TryGetLocalPath extraction).
- Client.UI-006: surface formerly silent catch blocks via an observable
  StatusMessage on the Subscriptions / Alarms VMs that bubbles up into
  the shell's status bar; soft fallbacks log at Information level so
  hard failures stay distinguishable.
- Client.UI-009: docs/Client.UI.md now lists Standard Deviation in the
  Aggregate row of the Query Options table.
- Client.UI-010: removed the unused MinDateTimeProperty /
  MaxDateTimeProperty styled properties from DateTimeRangePicker.
- Client.UI-011: updated the cert-store TextBox watermark from the
  legacy AppData/LmxOpcUaClient/pki to the canonical
  AppData/OtOpcUaClient/pki.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 11:25:20 -04:00
Joseph Doherty
59ecd18169 docs(code-reviews): regenerate index — 25 Low findings resolved
Batch 6 cleared Open findings in Driver.FOCAS.Cli (1 deferred to
Driver.Cli.Common), Driver.Cli.Common, Driver.Historian.Wonderware.Client,
Client.CLI, and Client.Shared.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 11:13:29 -04:00
Joseph Doherty
2a6ac07111 fix(client-shared): resolve Low code-review findings (Client.Shared-003,004,009,010,011)
- Client.Shared-003: DefaultSessionAdapter.WriteValueAsync / CallMethodAsync
  guard against null/empty Results and throw ServiceResultException with
  the response's ServiceResult code instead of indexing into a missing
  list.
- Client.Shared-004: DefaultSessionAdapter.CloseAsync / HistoryReadRawAsync
  / HistoryReadAggregateAsync use the Session.*Async overloads and honour
  the caller's CancellationToken.
- Client.Shared-009: AcknowledgeAlarmAsync returns the underlying
  ServiceResultException.StatusCode on failure instead of always Good;
  IOpcUaClientService doc updated to describe the new contract.
- Client.Shared-010: ConnectionSettings.CertificateStorePath defaults to
  empty; DefaultApplicationConfigurationFactory resolves the canonical
  PKI path lazily, so per-failover ConnectionSettings copies don't hit
  the filesystem.
- Client.Shared-011: added the alarm-fallback regression test, extracted
  EndpointSelector as a pure static, and added EndpointSelectorTests
  covering security-mode match, Basic256Sha256 preference, fallback,
  diagnostics, hostname rewrite, and null/empty guards.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 11:13:21 -04:00
Joseph Doherty
7fe9f16cf8 fix(client-cli): resolve Low code-review findings (Client.CLI-002,003,004,006,007,008,009,010)
- Client.CLI-002: SubscribeCommand's neverWentBad list now requires the
  node to be present in lastStatus (i.e. received at least one update)
  so the 'suspect' bucket only contains observed nodes.
- Client.CLI-003: every long-running command validates numeric option
  ranges (Interval / Depth / MaxDepth / Duration / Max) and throws
  CliFx CommandException on out-of-range values.
- Client.CLI-004: SubscribeCommand carries XML summary docs on the
  type, ctor, every [CommandOption] property, and ExecuteAsync —
  matching the sibling commands' style.
- Client.CLI-006: HistoryReadCommand parses --start / --end with
  InvariantCulture+UTC and surfaces FormatException as CommandException;
  every NodeIdParser.ParseRequired call wraps FormatException /
  ArgumentException as CommandException.
- Client.CLI-007: CommandBase.ConfigureLogging calls Log.CloseAndFlush()
  before assigning a new Log.Logger so prior sinks are disposed.
- Client.CLI-008: rewrote the subscribe and historyread sections of
  docs/Client.CLI.md (every flag documented, summary-bucket vocabulary,
  StandardDeviation aggregate, UTC --start/--end convention).
- Client.CLI-009: SubscribeCommand / AlarmsCommand use named local
  handlers and detach them via -= after UnsubscribeAsync so no
  notification reaches the console after the command's output phase
  ends.
- Client.CLI-010: added CommandRangeValidationTests,
  EventHandlerLifecycleTests, InputValidationErrorsTests,
  LoggerLifecycleTests, and SubscribeCommandSummaryTests pinning every
  Low fix; FakeOpcUaClientService gained AddDiscoveredVariable +
  RaiseDataChanged + BrowseResultsByParent helpers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 11:13:08 -04:00
Joseph Doherty
879925180b fix(driver-historian-wonderware-client): resolve Low code-review findings (Driver.Historian.Wonderware.Client-003,004,006,008,010)
- Driver.Historian.Wonderware.Client-003: replaced the mixed Interlocked
  + healthLock counters with RecordOutcome that touches _totalQueries
  and exactly one of _totalSuccesses / _totalFailures under one
  acquisition.
- Driver.Historian.Wonderware.Client-004: InvokeAndClassifyAsync routes
  transport + sidecar classification through a single RecordOutcome
  call; the legacy ReclassifySuccessAsFailure two-step is gone.
- Driver.Historian.Wonderware.Client-006: removed the dead
  ReconnectInitialBackoff / ReconnectMaxBackoff options and added a
  doc <remarks> stating the channel performs a single in-place
  reconnect; retry/backoff stays with the caller.
- Driver.Historian.Wonderware.Client-008: the audit-suppression comment
  block now records advisory titles, why neither applies, and the
  revisit trigger.
- Driver.Historian.Wonderware.Client-010: reworded Dispose() to claim
  deadlock-safety and added a GetHealthSnapshot summary documenting the
  single-channel collapse + counter invariant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 11:12:16 -04:00
Joseph Doherty
3ca569f621 fix(driver-cli-common): resolve Low code-review findings (Driver.Cli.Common-004,006)
- Driver.Cli.Common-004: confirm the FormatTable empty-input guard
  landed earlier (commit 1433a1c); flip status to Resolved with a
  cross-reference.
- Driver.Cli.Common-006: reword the SnapshotFormatter source-time
  column comment to describe the actual behaviour (right-most column,
  unmeasured, '-' for null timestamps) and confirm the
  DriverCommandBase summary now enumerates FOCAS.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 11:12:04 -04:00
Joseph Doherty
6923be3aa2 fix(driver-focas-cli): resolve Low code-review findings (Driver.FOCAS.Cli-001,002,003,004; -005 deferred)
- Driver.FOCAS.Cli-001: WriteCommand.ParseValue now wraps numeric
  FormatException / OverflowException as CliFx CommandException with
  the offending value.
- Driver.FOCAS.Cli-002: SubscribeCommand's OnDataChange handler and the
  banner both take a writeLock so notification-callback and main-thread
  writes can't interleave; handler exceptions are warn-and-swallow.
- Driver.FOCAS.Cli-003: FocasCommandBase.ValidateOptions rejects
  --cnc-port outside 1..65535, non-positive --timeout-ms, and
  non-positive --interval-ms; ExecuteAsync calls it first.
- Driver.FOCAS.Cli-004: 'await using var driver' is the sole driver
  disposal path; dropped the redundant explicit await ShutdownAsync.
- Driver.FOCAS.Cli-005 (Deferred): the fix lives in
  Driver.Cli.Common.SnapshotFormatter — explicitly naming the
  status-code shortlist there benefits every driver CLI. Left as a
  Driver.Cli.Common follow-up.
- Registered the new tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli.Tests
  project in ZB.MOM.WW.OtOpcUa.slnx.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 11:11:55 -04:00
Joseph Doherty
2a941b255f docs(code-reviews): regenerate index — 29 Low findings resolved
Batch 5 cleared Open findings in Driver.AbCip.Cli, Driver.AbLegacy.Cli,
Driver.S7.Cli, Driver.TwinCAT.Cli, and Driver.Modbus.Cli.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:35:12 -04:00
Joseph Doherty
80ef8806e0 fix(driver-modbus-cli): resolve Low code-review findings (Driver.Modbus.Cli-003,004,005,006,007,008)
- Driver.Modbus.Cli-003: ModbusCommandBase.ValidateEndpoint rejects
  --port outside 1..65535, non-positive --timeout-ms, and --unit-id
  outside 1..247.
- Driver.Modbus.Cli-004: wrapped SubscribeCommand's OnDataChange handler
  body in a try/catch (warn-and-swallow) and serialised the console
  write through a lock.
- Driver.Modbus.Cli-005: Probe / Read / Write now catch the
  cancellation-during-init OperationCanceledException and print
  'Cancelled.' instead of dumping a stack trace.
- Driver.Modbus.Cli-006: ProbeCommand.ComputeVerdict derives the headline
  from BOTH the driver state and the probe snapshot's OPC UA quality
  class so the headline can't disagree with the wire result.
- Driver.Modbus.Cli-007: docs/Driver.Modbus.Cli.md carries an explicit
  'CLI scope' callout — the address-string grammar is a DriverConfig
  JSON feature; the CLI takes the structured triple only.
- Driver.Modbus.Cli-008: pinned BuildOptions, ValidateEndpoint, the
  region-validation guards, ComputeVerdict, and the cancellation-during-
  initialize paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:35:05 -04:00
Joseph Doherty
f2ee027145 fix(driver-twincat-cli): resolve Low code-review findings (Driver.TwinCAT.Cli-001,002,003,004,005,006,007)
- Driver.TwinCAT.Cli-001: TwinCATCommandBase.Validate rejects
  non-positive TimeoutMs / IntervalMs and AmsPort outside 1..65535;
  ExecuteAsync calls it first.
- Driver.TwinCAT.Cli-002: SubscribeCommand serialises every WriteLine
  through a writeLock to remove the notification-callback vs banner
  interleave risk.
- Driver.TwinCAT.Cli-003: SubscribeCommand.DescribeMechanism derives
  the banner label from the returned ISubscriptionHandle.DiagnosticId
  so it can't disagree with what the driver actually did.
- Driver.TwinCAT.Cli-004: introduced TwinCATTagCommandBase carrying
  --poll-only + BuildOptions; BrowseCommand stays on the slimmer
  TwinCATCommandBase so --poll-only no longer surfaces in browse --help.
- Driver.TwinCAT.Cli-005: ProbeCommand --type now carries the 't' short
  alias to match the other commands.
- Driver.TwinCAT.Cli-006: 35 new tests covering Gateway / AmsAddress
  parse / BuildOptions / PollOnly / browse-helpers / probe-alias /
  mechanism derivation.
- Driver.TwinCAT.Cli-007: replaced the empty-init <inheritdoc/> with an
  explicit summary warning future maintainers about the no-op init.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:34:57 -04:00
Joseph Doherty
67ef6c4ebc fix(driver-s7-cli): resolve Low code-review findings (Driver.S7.Cli-004,005,006,007)
- Driver.S7.Cli-004: 'await using var driver' is the sole driver
  disposal path; dropped the redundant explicit await ShutdownAsync from
  each command's finally.
- Driver.S7.Cli-005: deleted the stale empty
  tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/ directory (the real test
  project lives under tests/Drivers/Cli/).
- Driver.S7.Cli-006: S7CommandBaseBuildOptionsTests cover the probe
  toggle, timeout mapping, host/port/CPU/rack/slot wiring, and tag list
  passthrough.
- Driver.S7.Cli-007: re-added the SubscribeCommand handler comment
  explaining the CliFx IConsole.Output usage and that the poll-thread
  raises events.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:34:48 -04:00
Joseph Doherty
f46e126208 fix(driver-ablegacy-cli): resolve Low code-review findings (Driver.AbLegacy.Cli-002,003,004,005,006,007)
- Driver.AbLegacy.Cli-002: WriteCommand.Value description lists the full
  true/false, 1/0, on/off, yes/no alias set.
- Driver.AbLegacy.Cli-003: SubscribeCommand serialises every WriteLine
  via a per-execution consoleGate lock so the poll-thread OnDataChange
  handler can't interleave with the banner.
- Driver.AbLegacy.Cli-004: dropped 'await using var driver' in favour of
  a plain 'var driver' + explicit await ShutdownAsync in finally; the
  driver is no longer shut down twice.
- Driver.AbLegacy.Cli-005: SubscribeCommand.IntervalMs description
  carries the PollGroupEngine 250ms-floor caveat; docs/Driver.AbLegacy.Cli.md
  spells out the same.
- Driver.AbLegacy.Cli-006: ProbeCommand --type now carries the short
  alias 't' to match the other commands.
- Driver.AbLegacy.Cli-007: BuildOptionsTests cover the probe-disabled,
  device-shape, tag-passthrough, timeout-propagation, and empty-tag-list
  paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:34:32 -04:00
Joseph Doherty
759af8c1bb fix(driver-abcip-cli): resolve Low code-review findings (Driver.AbCip.Cli-003,004,005,006,007,008)
- Driver.AbCip.Cli-003: SubscribeCommand prints the 'Subscribed' banner
  BEFORE wiring OnDataChange so the main thread can't interleave its
  write with the poll-thread handler.
- Driver.AbCip.Cli-004: AbCipCommandBase.Timeout and SubscribeCommand
  validate TimeoutMs / IntervalMs and throw CommandException on
  non-positive values.
- Driver.AbCip.Cli-005: every command now calls FlushLogging() in its
  finally block.
- Driver.AbCip.Cli-006: Timeout init throws NotSupportedException with a
  pointer at TimeoutMs instead of silently swallowing assignments.
- Driver.AbCip.Cli-007: added AbCipCommandBaseTests covering BuildOptions
  shape, probe / controller-browse / alarm toggles, host address, family
  selection, tag list passthrough.
- Driver.AbCip.Cli-008: rewrote the opening paragraph in
  docs/Driver.AbCip.Cli.md to credit the six-CLI roster with a pointer
  at docs/DriverClis.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:34:25 -04:00
Joseph Doherty
61c0311938 docs(code-reviews): regenerate index — 24 Low findings resolved
Batch 4 cleared Open findings in Driver.TwinCAT, Driver.Modbus,
Driver.OpcUaClient, Driver.Historian.Wonderware, and Driver.Modbus.Addressing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:18:21 -04:00
Joseph Doherty
9263519852 fix(driver-modbus-addressing): resolve Low code-review findings (Driver.Modbus.Addressing-006,007,009)
- Driver.Modbus.Addressing-006: broaden the catch in TryParseFamilyNative
  so a future helper throwing a non-Argument/Overflow type still satisfies
  the try-parse contract.
- Driver.Modbus.Addressing-007: document that the address grammar does
  not carry ModbusStringByteOrder (the structured-tag path does);
  add a 'Grammar scope' bullet to docs/v2/dl205.md.
- Driver.Modbus.Addressing-009: reword the ModbusModiconAddress comments
  so they don't imply a leading-digit invariant the parser doesn't
  enforce.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:18:15 -04:00
Joseph Doherty
1f29b215c8 fix(driver-historian-wonderware): resolve Low code-review findings (Driver.Historian.Wonderware-004,005,007,008,010,011,012)
- Driver.Historian.Wonderware-004: ToHistorianEvent synthesises a fresh
  Guid when the upstream EventId is unparseable and logs the substitution
  instead of writing the historian with Guid.Empty.
- Driver.Historian.Wonderware-005: GetHealthSnapshot derives the
  connection-open booleans from the active-node fields so the snapshot
  is self-consistent without depending on the secondary lock.
- Driver.Historian.Wonderware-007: SID-mismatch branch in PipeServer now
  sends a HelloAck { Accepted=false, RejectReason } so the client sees a
  symmetric rejection.
- Driver.Historian.Wonderware-008: classify StartQuery failures —
  connection-class codes drop the connection, query-class codes throw
  QueryClassStartQueryException so the IPC layer surfaces Success=false.
- Driver.Historian.Wonderware-010: RequestTimeoutSeconds now enforced
  via BuildRequestCts linked to the caller's CancellationToken.
- Driver.Historian.Wonderware-011: refreshed XML docs to describe the
  current sidecar / named-pipe architecture (Galaxy.Host / Proxy
  references reframed as historical context).
- Driver.Historian.Wonderware-012: pinned the previously-uncovered
  HistorianDataSource behaviours with five new test files; also removed
  the stale empty tests/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests
  directory.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:18:10 -04:00
Joseph Doherty
42aa82de29 fix(driver-opcuaclient): resolve Low code-review findings (Driver.OpcUaClient-011,014)
- Driver.OpcUaClient-011: rewrote the ValueRank comment with the OPC UA
  Part 3 constants and an explicit scalar/array boundary at
  valueRank >= 0.
- Driver.OpcUaClient-014: track every MonitoredItem.Notification handler
  in a MonitoredItemNotificationHandle record; UnsubscribeAsync /
  UnsubscribeAlarmsAsync / ShutdownAsync detach the handler before
  Subscription.DeleteAsync so the SDK's invocation list no longer keeps
  the driver alive.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:17:55 -04:00
Joseph Doherty
d5322b0f9a fix(driver-modbus): resolve Low code-review findings (Driver.Modbus-003,007,008,009,010,011,012)
- Driver.Modbus-003: route every _health access through ReadHealth /
  WriteHealth helpers backed by Volatile.Read / Volatile.Write so a
  burst of concurrent ReadAsync callers always sees a complete snapshot.
- Driver.Modbus-007: promoted the Int64 / UInt64 → Int32 surfacing
  caveat to a full <remarks> block; rewrote DisableFC23's doc to flag it
  as reserved / no-op.
- Driver.Modbus-008: deleted stale duplicate doc, rewrote the
  prohibition-block summaries to credit the shipped re-probe loop, and
  removed the unused 'status' local in the ModbusException catch arm.
- Driver.Modbus-009: bind-time validation rejects StringLength < 1 for
  String tags; ModbusTcpTransport clamps keep-alive intervals to whole
  seconds (>=1).
- Driver.Modbus-010: documented WriteOnChangeOnly's cache-invalidation
  policy (reads-only) and the write-only-tag caveat.
- Driver.Modbus-011: collected the scattered instance fields into a
  single contiguous block at the top of ModbusDriver.
- Driver.Modbus-012: covered the previously-uncovered Reinitialize
  state-hygiene, malformed/truncated/empty-bitmap response, and
  DisposeAsync teardown paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:17:51 -04:00
Joseph Doherty
3c75db7eb6 fix(driver-twincat): resolve Low code-review findings (Driver.TwinCAT-004,006,014,015,016)
- Driver.TwinCAT-004: corrected the IEC time-type inline comments;
  documented that the driver currently surfaces them as raw UInt32
  counters.
- Driver.TwinCAT-006: ResolveHost returns a documented UnresolvedHost
  sentinel when no devices are configured instead of returning the
  logical DriverInstanceId (which never matches GetHostStatuses).
- Driver.TwinCAT-014: wired Probe.Timeout into the probe-loop call and
  added a NotificationMaxDelayMs config knob threaded through
  AddNotificationAsync.
- Driver.TwinCAT-015: Dispose() runs a genuinely synchronous teardown
  with bounded waits (no sync-over-async deadlock pattern).
- Driver.TwinCAT-016: pinned the Structure-tag rejection and the
  probe-loop vs read disposal race with regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:17:42 -04:00
Joseph Doherty
bccff1339d docs(code-reviews): regenerate index — 22 Low findings resolved
Batch 3 cleared Open findings in Driver.Galaxy, Driver.AbCip,
Driver.AbLegacy, Driver.FOCAS, and Driver.S7.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:45:53 -04:00
Joseph Doherty
af0f09d07e fix(driver-s7): resolve Low code-review findings (Driver.S7-003,005,009,010,013)
- Driver.S7-003: ArgumentNullException.ThrowIfNull on the references
  argument at the top of ReadAsync / WriteAsync (was reaching .Count
  before any null check).
- Driver.S7-005: drop the redundant global::S7.Net.Plc qualifiers in
  ReadOneAsync / WriteOneAsync — using S7.Net already covers Plc.
- Driver.S7-009: PollLoopAsync degrades _health to Degraded after
  sustained failure and backs off exponentially up to PollBackoffCap;
  resets on a healthy tick so an operator can see the loop wedge.
- Driver.S7-010: Dispose runs the synchronous teardown directly with a
  bounded WhenAll Wait drain instead of bridging via DisposeAsync().
- Driver.S7-013: reject unsupported S7DataType values (Int64 / UInt64 /
  Float64 / String / DateTime) at InitializeAsync so half-implemented
  types no longer leak BadNotSupported live nodes into the address space.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:45:45 -04:00
Joseph Doherty
6575c6e5f6 fix(driver-focas): resolve Low code-review findings (Driver.FOCAS-007,008,009,010,011)
- Driver.FOCAS-007: optional ILogger<FocasDriver> + alarm-projection
  logger; log Debug around every formerly-empty catch (probe / shutdown
  / fixed-tree / recycle / alarms-read / projection).
- Driver.FOCAS-008: cache the parsed FocasAddress per tag at
  InitializeAsync; Read/WriteAsync look it up instead of re-parsing on
  every call.
- Driver.FOCAS-009: ProbeLoopAsync now wraps client.ProbeAsync in a
  linked CTS honouring Probe.Timeout so a hung CNC socket can't block
  past the configured limit.
- Driver.FOCAS-010: FocasOperationModeExtensions.ToText delegates to
  FocasOpMode.ToText — single canonical op-mode label surface.
- Driver.FOCAS-011: FocasAlarmType constants are typed short to match
  the cnc_rdalmmsg2 wire field and the projection switch arms.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:45:38 -04:00
Joseph Doherty
f7e3e9885e fix(driver-ablegacy): resolve Low code-review findings (Driver.AbLegacy-005,011,013)
- Driver.AbLegacy-005: optional ILogger<AbLegacyDriver> ctor parameter,
  logged init failure / probe transitions / first non-zero libplctag
  status per device.
- Driver.AbLegacy-011: Dispose() runs the synchronous teardown directly
  instead of bridging via DisposeAsync().AsTask().GetAwaiter().GetResult()
  to remove the documented sync-over-async deadlock pattern.
- Driver.AbLegacy-013: documented the ResolveHost three-tier fallback
  chain in XML and pointed DiscoverAsync's IsArray=false comment at the
  Modbus ArrayCount pattern for the eventual multi-element follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:45:31 -04:00
Joseph Doherty
77b8686199 fix(driver-abcip): resolve Low code-review findings (Driver.AbCip-007,011,012,013,015)
- Driver.AbCip-007: inject an optional ILogger<AbCipDriver> /
  ILogger<AbCipAlarmProjection> (default NullLogger) and log around
  every read / write / template-fetch / probe / alarm-poll failure path.
- Driver.AbCip-011: LogWarning when InitializeAsync is configured with
  Probe.Enabled=true but ProbeTagPath is blank — operators now see why
  GetHostStatuses keeps reporting Unknown.
- Driver.AbCip-012: documented the LibplctagTemplateReader per-call
  Tag cost as accepted given libplctag's own connection pool and the
  low-frequency discovery use-case.
- Driver.AbCip-013: per-device AllowPacking + ConnectionSize overrides
  on AbCipDeviceOptions, threaded through AbCipTagCreateParams; central
  BuildCreateParams helper replaces five ad-hoc clones; AllowPacking
  now reaches Tag.AllowPacking at runtime.
- Driver.AbCip-015: stale-comment sweep — every PR-N forward-reference
  is rewritten to describe present behaviour.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:45:19 -04:00
Joseph Doherty
9f7ae20995 fix(driver-galaxy): resolve Low code-review findings (Driver.Galaxy-005,010,012,013)
- Driver.Galaxy-005: rewrite the EventPump BoundedChannelOptions comment
  to honestly describe the Wait+TryWrite pattern.
- Driver.Galaxy-010: ResolveApiKey now warns when a literal API key is
  used in production wiring; added an explicit dev: prefix for known
  cleartext-in-dev cases and rewrote the GalaxyGatewayOptions doc.
- Driver.Galaxy-012: O(1) reverse-lookup for SubscriptionRegistry
  dispatch via per-entry FullRefByItemHandle map; immutable hash-set for
  the cross-binding reverse map; SubscribeAsync / ReadViaSubscribeOnce
  use BuildResultIndex for per-reference correlation.
- Driver.Galaxy-013: ReinitializeAsync now validates the incoming JSON
  against the running options; ReplayOnSessionLost honoured by the
  Replay path; class summary rewritten to describe the shipped surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:45:08 -04:00
Joseph Doherty
5c513f99fd docs(core-scripting): mark Core.Scripting-008 as Won't Fix (documented limitation)
The Resolution prose was already recorded under Core.Scripting commit
(0454822); status was left as Open. Flip to Won't Fix to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:25:05 -04:00
Joseph Doherty
2580b5026f docs(code-reviews): regenerate index — 27 Low findings resolved
Batch 2 cleared Open findings in Core.ScriptedAlarms, Core.Scripting,
Core.VirtualTags, Admin, and Server (Core.ScriptedAlarms-009 documented
under Won't Fix per the recommendation).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:24:27 -04:00
Joseph Doherty
6134050ceb fix(server): resolve Low code-review findings (Server-004,006,008,012,014,015)
- Server-004: pass the role-derived display name to UserIdentity's base
  ctor (the SDK's DisplayName has no public setter) and drop the dead
  Display property; make RoleBasedIdentity internal sealed.
- Server-006: derive a bounded CancellationToken from the SDK's
  OperationContext.OperationDeadline in OnReadValue / OnWriteValue so a
  stalled driver call can no longer pin the request thread.
- Server-008: mark handled slots via CallMethodRequest.Processed = true
  in RouteScriptedAlarmMethodCalls (the SDK skips on Processed, not on a
  Good error slot).
- Server-012: PeerHttpProbeLoop.ProbeAsync stops mutating client.Timeout
  per call; uses a per-request CancellationTokenSource linked to the
  shutdown token instead.
- Server-014: wire SealedBootstrap into Program.cs via AddSealedBootstrap
  + OpcUaServerService so the generation-sealed cache + stale-config flag
  + resilient reader actually run; /healthz now reflects cache-fallback
  state.
- Server-015: replace the stale 'PR 16 / PR 17 minimum-viable scope'
  class summaries on OtOpcUaServer and OpcUaServerOptions with the
  shipped LDAP + anonymous-role + configurable security-profile prose.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:24:20 -04:00
Joseph Doherty
2b33b64a58 fix(admin): resolve Low code-review findings (Admin-010,011,012)
- Admin-010: vendor Bootstrap 5.3.3 (CSS + JS bundle + maps + provenance
  README) under wwwroot/lib/bootstrap and reference local paths from
  App.razor — Admin no longer pulls Bootstrap from jsDelivr.
- Admin-011: swap FleetStatusPoller's three plain dictionaries for
  ConcurrentDictionary so ResetCache can't race a poll tick.
- Admin-012: drop the EquipmentId column from EquipmentCsvImporter (per
  admin-ui.md — equipment id is system-derived from EquipmentUuid);
  EquipmentImportBatchService and the textarea placeholder updated to
  match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:24:07 -04:00
Joseph Doherty
3f01a24b45 fix(core-virtual-tags): resolve Low code-review findings (Core.VirtualTags-004,006,007,009,010,011,013)
- Core.VirtualTags-004: CoerceResult now covers every scalar
  DriverDataType and throws on the default arm; Load rejects unsupported
  declared types.
- Core.VirtualTags-006: Subscribe/Unsub prune empty observer-list
  entries from _observers under the same lock with a reconfirm-on-add
  race guard.
- Core.VirtualTags-007: rewrote TimerTriggerScheduler so each TickGroup
  tracks an InFlight flag (Interlocked CAS); ticks that overlap a still-
  running tick for the same group are skipped + counted.
- Core.VirtualTags-009: DirectDependencies / DirectDependents return a
  shared static empty set on miss instead of allocating per call.
- Core.VirtualTags-010: corrected XML docs to reference the real engine
  symbols (OnUpstreamChange, CascadeAsync, etc.) instead of phantom types.
- Core.VirtualTags-011: Load now rejects scripts whose declared Writes
  target a non-registered virtual-tag path.
- Core.VirtualTags-013: DependencyCycleException renders SCC members as
  a set rather than a fabricated arrow-traversal edge path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:23:53 -04:00
Joseph Doherty
0a20de728d fix(core-scripting): resolve Low code-review findings (Core.Scripting-005,006,008,009,011)
- Core.Scripting-005: DependencyExtractor.HandleTagCall now recognises
  raw-string literal paths by checking the StringLiteralExpression node
  kind instead of the legacy StringLiteralToken kind.
- Core.Scripting-006: scope CompiledScriptCache failed-compile eviction
  with TryRemove(KeyValuePair) so a racing retry entry is not evicted.
- Core.Scripting-008: document the per-publish assembly accretion as an
  accepted limitation in docs/VirtualTags.md.
- Core.Scripting-009: enumerate the authoritative deny-list (namespace
  prefixes + type-granular denies) in the Phase 7 decision-#6 entry to
  match ForbiddenTypeAnalyzer.
- Core.Scripting-011: pin ScriptSandbox.Build, ScriptContext.Deadband
  boundary semantics, and end-to-end factory + companion-sink
  integration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:23:42 -04:00
Joseph Doherty
99354bfaf2 fix(core-scripted-alarms): resolve Low code-review findings (Core.ScriptedAlarms-003,006,008,010,011; -009 documented)
- Core.ScriptedAlarms-003: emit OnEvent OUTSIDE _evalGate by collecting
  pending emissions during the gate-held section and flushing them after
  release; eliminates re-entrancy deadlock the docs already promised.
- Core.ScriptedAlarms-006: track every fire-and-forget Reevaluate /
  ShelvingCheck task in _inFlight; Dispose drains the set so the engine
  no longer races store writes against teardown.
- Core.ScriptedAlarms-008: store comments as ImmutableList<AlarmComment>
  so AppendComment is O(log n) instead of O(n).
- Core.ScriptedAlarms-010: document the deliberate input-quality
  asymmetry (Uncertain drives the predicate, renders {?} in the message)
  in docs/ScriptedAlarms.md and on MessageTemplate.Resolve remarks.
- Core.ScriptedAlarms-011: propagate the no-op reason through
  TransitionResult.NoOp(state, reason) and log it from
  ScriptedAlarmEngine.ApplyAsync.
- Core.ScriptedAlarms-009 (Won't Fix per recommendation): documented the
  per-evaluation dictionary allocation in docs/v2/Galaxy.Performance.md
  with a mitigation path if a future soak surfaces pressure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 07:23:31 -04:00
Joseph Doherty
e74e8f7b31 docs(code-reviews): regenerate index — 23 Low findings resolved
Batch 1 cleared Open findings in Core, Core.Abstractions, Core.AlarmHistorian,
Configuration, and Analyzers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 05:39:16 -04:00
Joseph Doherty
0993fa5a19 fix(analyzers): resolve Low code-review findings (Analyzers-002,003,004,005,007)
- Analyzers-002: drop the three dead AlarmSurfaceInvoker entries from
  the wrapper-method allow-list and from the diagnostic message.
- Analyzers-003: bail out of AnalyzeInvocation when the semantic model
  is null (was previously emitting a false positive).
- Analyzers-004: resolve guarded-interface + wrapper-method symbols
  once via CompilationStartAction and compare with SymbolEqualityComparer
  instead of formatting fully-qualified names on every invocation.
- Analyzers-005: add regression tests for default-interface-method
  reads (ReadAtTimeAsync / ReadEventsAsync on a concrete driver), with
  + without an override, and inside a CapabilityInvoker.ExecuteAsync
  lambda.
- Analyzers-007: rewrite the analyzer remarks to accurately describe
  the symbol-identity guarded-call detection, DIM handling, and the
  wrapper-lambda match heuristic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 05:38:37 -04:00
Joseph Doherty
0da4f3b63a fix(core-alarm-historian): resolve Low code-review findings (Core.AlarmHistorian-008,011)
- Core.AlarmHistorian-008: cache queue depth in an Interlocked counter so
  EnqueueAsync no longer runs COUNT(*) on every alarm; consolidate
  DrainOnceAsync onto a single SqliteConnection per tick (purge, batch
  read, dead-letter, and outcome transaction all share it).
- Core.AlarmHistorian-011: confirm the stale Galaxy.Host XML doc
  references were already fixed under earlier commits; flip to Resolved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 05:38:26 -04:00
Joseph Doherty
b92fea15d4 fix(configuration): resolve Low code-review findings (Configuration-004,005,007,010,011)
- Configuration-004: NodePermissions stored as int to match the EF
  HasConversion<int>() in OtOpcUaConfigDbContext.ConfigureNodeAcl.
- Configuration-005: serialise LiteDbConfigCache.PutAsync so concurrent
  Put for the same (ClusterId, GenerationId) cannot duplicate rows.
- Configuration-007: rethrow OperationCanceledException from
  GenerationApplier.ApplyPass when the caller's token is cancelled.
- Configuration-010: scrub secrets and drop the full exception object
  from the ResilientConfigReader fallback warning log.
- Configuration-011: pin the previously-uncovered GenerationApplier
  cancellation and path-length / publish-validation paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 05:38:18 -04:00
Joseph Doherty
8be6afbda4 fix(core): resolve Low code-review findings (Core-004,008,009,010,011,012)
- Core-004: add ConfigureAwait(false) to DriverHost.RegisterAsync /
  UnregisterAsync / DisposeAsync.
- Core-008: rewrite the BuildAddressSpaceAsync XML doc to correctly name
  the caller (OpcUaApplicationHost.PopulateAddressSpaces) that owns the
  per-driver isolation.
- Core-009: snapshot DriverResilienceOptions once per non-idempotent write
  in CapabilityInvoker.ExecuteWriteAsync.
- Core-010: switch DriverResilienceOptions.Resolve to TryGetValue with a
  diagnostic error message when a tier table is missing a capability.
- Core-011: add an optional diagnostic callback to PermissionTrieBuilder
  so production callers can surface scope-path mismatches.
- Core-012: correct the stale WedgeDetector ctor summary and add the
  Reconnecting row to DriverHealthReport's state matrix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 05:38:09 -04:00
Joseph Doherty
ff2e75ab98 fix(core-abstractions): resolve Low code-review findings (Core.Abstractions-004,005,006,007,008)
- Core.Abstractions-004: guard DriverTypeRegistry.Register with a Lock so
  concurrent registrations are atomic.
- Core.Abstractions-005: narrow PollGroupEngine catch blocks to non-fatal
  exceptions, add optional onError callback, tolerate disposed-CTS races.
- Core.Abstractions-006: document the deliberate int-vs-uint asymmetry on
  IHistoryProvider.ReadEventsAsync / IHistorianDataSource.ReadEventsAsync.
- Core.Abstractions-007: pin the gaps with PollGroupEngine + DriverHealth
  contract tests.
- Core.Abstractions-008: correct XML docs on DriverHealth.LastError and
  the optional / required asymmetry on the history-read surfaces.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 05:37:54 -04:00
Joseph Doherty
a02c0ffe36 docs(code-reviews): record Admin-013 (SignalR hub clients cannot authenticate)
Records the post-review finding discovered during browser smoke-testing: the
Admin-003 hub hardening was incomplete — the server-side Blazor HubConnection
clients had no way to authenticate, so hub negotiate 401'd and four cluster
pages threw unhandled 500s. Logged as Admin-013 (High, Error handling &
resilience), Status Resolved, fixed by commits f254539 + 8d5dbb4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 12:29:36 -04:00
Joseph Doherty
8d5dbb46f2 fix(admin): authenticate SignalR hub clients with a bearer-token scheme
The Admin-003 fix gated every SignalR hub with [Authorize], but the server-side
Blazor HubConnection clients had no way to authenticate: the browser's HttpOnly
auth cookie is not reachable from the interactive circuit, so every hub negotiate
returned 401 and the Admin live-update feature was non-functional app-wide
(silently degraded on Hosts/ScriptLog, fatal on the cluster pages).

Introduce a token-based hub auth path:
- HubTokenService mints/validates short-lived tokens using ASP.NET Core Data
  Protection (the same primitive that protects the auth cookie — no signing-key
  management, no new packages). Tokens carry the user's name + roles.
- HubTokenAuthenticationHandler is a custom "HubToken" auth scheme that reads the
  token from the Authorization: Bearer header (negotiate) or the access_token
  query parameter (WebSocket upgrade).
- The "HubClients" authorization policy runs both the cookie and HubToken
  schemes; the hub endpoints use RequireAuthorization("HubClients").
- AdminHubConnectionFactory builds hub connections with an AccessTokenProvider
  that mints a fresh token for the circuit's authenticated user on every
  (re)connect. All six hub-consuming pages now resolve connections through it.

Hub negotiate now returns 200 and the WebSocket upgrades (101); live updates
work. The best-effort try/catch guards added previously are kept as defence.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 12:06:29 -04:00
Joseph Doherty
f2545392e0 fix(admin): stop SignalR hub-connect failure from crashing cluster pages
The Admin-003 fix gated every SignalR hub with [Authorize]/RequireAuthorization,
but the server-side HubConnection clients on ClusterDetail, AclsTab, RedundancyTab
and RoleGrants cannot forward the browser's HttpOnly auth cookie — so the hub
negotiate returns 401. Those four pages called HubConnection.StartAsync()
unguarded, so the 401 surfaced as an unhandled exception (a 500 page for the
prerendered ClusterDetail, a broken circuit for the others).

Wrap StartAsync/SendAsync in try/catch on all four, matching the established
best-effort pattern already used in Hosts.razor and ScriptLog.razor: the live
banner / live refresh degrades but the page renders. Restoring functional hub
live-updates needs a token-based hub auth scheme (cookie forwarding is not
viable across the prerender/interactive boundary) and is left as follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 11:56:06 -04:00
Joseph Doherty
bbe292a4b4 docs(code-reviews): regenerate index — 126 Medium findings resolved
All Medium-severity code-review findings across the 29 reviewed modules
are now Resolved. The Pending findings table holds only Low-severity items.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 11:29:21 -04:00
Joseph Doherty
0f3b74ad87 fix(server): wire PermissionTrieCache into AuthorizationGate for generation pinning
Core-002 fixed TriePermissionEvaluator to evaluate each request against
the session's bound AuthGenerationId rather than whatever the cache
currently holds. AuthorizationGate.BuildSessionState was not updated at
the same time: it hardcoded AuthGenerationId = 0, so the evaluator's
GetTrie(cluster, 0) call returned null for any generation != 0, causing
every gated operation to silently fail with NotGranted regardless of
actual grants. The 42 gate/matrix/deferred-hardening tests all started
failing as a result.

Fix: add an optional PermissionTrieCache parameter to AuthorizationGate;
BuildSessionState now stamps AuthGenerationId from the cache's current
generation for the session's cluster. AuthorizationBootstrap.BuildGateAsync
passes the cache it creates. All 7 test MakeGate helpers updated to pass
the cache so tests produce a valid AuthGenerationId. 433/433 server tests
now pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 11:25:39 -04:00
Joseph Doherty
7bf2dc49cf fix(driver-twincat): align status-mapper tests with corrected ADS codes (Driver.TwinCAT-011)
The Driver.TwinCAT-011 fix rewrote TwinCATStatusMapper with correct
numeric values from Beckhoff.TwinCAT.Ads 7.0.172 (e.g. DeviceSymbol-
VersionInvalid = 1809 / 0x0711, not 1794 / 0x0702). Pre-existing
StatusMapper_covers_known_ads_error_codes InlineData cases were written
against the old wrong mappings and now fail; StatusMapper_recognises_
symbol_version_changed_code asserted the legacy 0x0702 constant. Update
both test files to match the corrected mapper and add a comment
documenting the correction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 11:25:25 -04:00