Review of 75e559f6:
- OnConditionReportEvent now mirrors MonitoredNode2's AuditEventState guard (only while Server.Auditing,
over SignAndEncrypt / https-uabinary). Conditions report their Part 9 method audit events through
themselves, so the first cut queued AuditConditionAcknowledge (i=8944) etc. to every i=2253 item with
auditing off -- the removed folder->Server forward used to drop them.
- ReconcileAllEventsMonitoring keeps one AreEventsMonitored unit on every materialised condition while any
i=2253 item exists (called from SubscribeToAllEvents, materialise, removal, rebuild), so
ConditionState.EventsMonitored() is true on a server whose only subscribers are Server-object items.
A protected SubscribeToEvents override no-ops all-events items, which stops the SDK's AddRootNotifier
(unsubscribe: true bug) taking that unit back from conditions rebuilt under a live item.
Flag-off over-the-wire coverage: Acknowledge/AddComment with and without a folder co-subscriber (no audit
event, transitions once each), EventsMonitored on/after-rebuild/off, rebuild survival with one copy per
event, native + scripted conditions once each, refresh once per condition with RefreshStart/End; plus an
ACL-on native-vs-scripted notifier-folder decision. Falsified: removing the audit guard reds both audit
cases; removing the reconcile reds the monitored test.
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
Server-object (all-events) items bypassed the ACL entirely: the SDK attached them to every root
notifier's fan-out via SubscribeToAllEvents, and a root notifier with no direct subscriber forwarded
to the core Server object, which this node manager never sees.
- SubscribeToAllEvents override records the item and attaches it to nothing.
- Each condition's own OnReportEvent (fires once per event) queues the event to each recorded item
whose session holds Subscribe on the condition (RawPath) or any notifier folder it reports through
-- the per-folder path's question; otherwise mirrors MonitoredNode2.OnReportEvent.
- The root-notifier forward to the core Server object (OnReportEvent override) is dropped.
- ConditionRefresh on an i=2253 item is filtered the same way (once per condition).
- Revalidation is implicit: every event re-asks the authorizer (per-generation cache, key expiry and
revocation re-checked per call). Inert with Security:Acl:Enabled=false.
Over-the-wire tests: no-grant vs granted (positive control), created-before-deploy, revoke/restore by
deployment, key expiry with no tick, refresh, and flag-off. Falsified: disabling the filter, or
restoring the forward, turns the enforcement tests red.
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
#562: direct event (alarm) monitored items are now tracked in a second map
(_eventItems, fed by the SubscribeToEvents override) and re-authorized by
RevalidateMonitoredItems on both triggers (DataPlaneAuthorityState.Changed and
AclRevalidationTimer). A denied item is detached via the SDK's own protected
SubscribeToEvents(unsubscribe: true) - never MonitoringMode.Disabled - and
re-attached on restore; a client delete of a stood-down item is absorbed.
Over-the-wire tests: revoke -> stop (positive control on a still-granted
notifier) -> restore -> resume; clock-driven expiry via the real timer with a
control session; delete-while-stood-down is not resurrected; unexpired
negative control. Server-object (all-events) subscriptions are documented as
outside the ACL (observed over the wire).
#563: DataPlaneAuthorizer.AuthorizeKeyUsable runs the FindUsableKey probe on
the WriteOperate and AlarmAck role gates regardless of Security:Acl:Enabled,
so an expired or deployment-disabled key loses its activation-time roles on
its open session. Anonymous (no key id) is unaffected; reads/subscriptions
stay ungated with the flag off. No new flag.
Docs: docs/security.md and the CLAUDE.md API-key/ACL section.
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
ConfigureSharedLogger had no production caller, so the pool's only
diagnostics (first-wins options conflicts, scheduler/close failures)
went to NullLogger. DriverFactoryBootstrap now configures it from the
driver registry factory and from the AbCip/AbLegacy probe factories
(admin-only nodes reach the pool only through Test Connect). Wiring tests
reset the pool to a null logger first, so removing the call goes red
(verified).
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
The old AdminUI cannot author the per-driver transport key (#576), so the
'pin before upgrading' recipe had no working path. A node-level appsetting
(env Drivers__AbCip__DefaultTransport) now supplies the default for AbCip
drivers with no key: explicit key > node default > compiled Native. Bound
and validated unconditionally (bad value fails host start); the driver
factory and the Test Connect probe both read it so they agree. AbCip.md
upgrade recipe rewritten around the key; Configuration.md documents it.
Also documents first-wins for AllowPacking/probe and host-alias throttles
(final review M6) and moves the status-bit note out of AbCip.md (M5).
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
On a wire the native PCCC path is proven only for N-file reads/writes and
one F8 read (ab_server). An absent transport key now resolves to Libplctag
for every family; explicit Native still wins and still warns once per
instance on PLC-5/LogixPccc. AdminUI dropdown, CLI help, evidence claims
(PcccFileType, AbLegacy.md wire-evidence table, release note) corrected:
S:0 (default probe) and B/T/C/R/S/L/ST/A/O/I are unverified natively.
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
Task 22 (spec section 8 P5), with the controller rulings that exclude PLC-5 and LogixPccc.
- Transport is nullable on both option records; null (key absent) means "use the default" and is
never serialized. Factory DTO parse: absent -> null; a bad value still fails via ParseEnum.
- One resolver per driver (AbCipTransportResolver / AbLegacyTransportResolver), used by the driver
constructor, the Test Connect probe and (through the driver) the CLI.
AbCip: null -> Native. AbLegacy: null -> Native unless any device is Plc5 or LogixPccc, then the
whole driver resolves to Libplctag. Explicit values always win; explicit Native with a
Plc5/LogixPccc device logs one Warning at init.
- Init log shows configured vs effective, e.g. transport=Native (default).
- AdminUI forms: "Default (...)" / "Native (managed)" / "libplctag (fallback)". Default omits the
key on save (reverses Task 19's explicit re-emit, which would have pinned every edited driver).
- CLIs: --transport default|native|libplctag, default "default" via the resolver, so
--plc-type Plc5 with no --transport runs libplctag.
- Tests pinned to Libplctag where a default-path runtime was created: found by instrumenting the
native factories with the current xUnit test name (one unit test, AbLegacyReinitConfigAdoption;
plus the two fixture-gated AbCip probe E2E tests whose exact messages were proven on libplctag).
TransportMatrix and the differential harness already pass explicit transports.
- Switch/resolver/CLI/form tests are falsified by mutating the resolver (default -> Libplctag,
exclusion removed, AbLegacy default -> Libplctag): each mutation turns them red.
- Replaces the inline role-name literal in AbCipDriverFactoryExtensions' ParseEnum call with a named
constant; RoleAuthorityGuardTests was already red on the base commit for that line.
- Docs: AbCip.md / AbLegacy.md default + release note; CLAUDE.md AB section.
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
- AbCip/AbLegacy driver forms gain a Transport InputSelect; FromOptions/ToOptions now carry
Transport, so saving any other field no longer silently reverts Native to libplctag.
- Both drivers log the constructed transport once per init.
- AbCip factory now takes the host ILoggerFactory (it ran on NullLogger in production, hiding
every AbCip log line); DriverFactoryBootstrap passes it.
- Gate results: AdminUI /run (T19) native, inverted (dead port) and libplctag legs.
BoundedApiKeyStore bounded ConfigDbApiKeyStore with a
CancellationTokenSource(TimeSpan, TimeProvider) linked into the inner call. That
bounds the call only if the callee chooses to observe the token, and
Microsoft.Data.SqlClient does not while a connect/login is in flight on a
black-holed socket. The 2026-08-20 live gate paused the SQL container and watched
a keyed activation hang 42.9 s past its 2000 ms bound — the "did not answer
within 2000 ms" Warning fired only at unpause — and because this runs inside the
SDK's synchronous SessionManager-lock-holding impersonation callback, an
unrelated already-open subscription stalled 57 s behind it. That is precisely the
hazard the bound exists to prevent, and the TTL cache cannot help a miss.
BoundAsync now races the inner call against Task.Delay on the injected clock with
Task.WhenAny. Inner wins: stop the timer, await it, faults and genuine caller
cancellation propagate unchanged. Delay wins: cancel the linked token anyway (a
cooperative inner store should still stop), hand the abandoned task to
ObserveAbandoned, meter outcome=timeout, log the now-honest Warning, and throw
OperationCanceledException — the same exception the authenticator's catch-all
already maps to the standard opaque denial. Fail-closed is unchanged: nothing
here can admit.
ObserveAbandoned disposes the CTS and TOUCHES finished.Exception (the observation
itself), so a late SqlClient fault can never become an UnobservedTaskException.
Debug, not Warning — the timeout is already reported at Warning, and an outage
must not produce two lines per activation for one event.
An abandoned answer is NEVER cached: it was taken before the deadline and would
be served after it, so a key revoked or expired in the meantime could be admitted
from a lookup nobody was still waiting for. StoreTimeoutMs <= 0 still
short-circuits to the unbounded call byte-for-byte (the documented rollback).
RED BEFORE GREEN. Every pre-existing deadline test drives GatedApiKeyStore, whose
waits take the token, so a cooperative CancelAfter unblocks them and all 15
passed against the defective build — the suite could not see the gate's failure
at all. The new UncooperativeApiKeyStore ignores ct entirely and completes only
when the test releases it. Against the unmodified store the three new tests fail
3/18 with "should throw OperationCanceledException but threw TimeoutException"
under a test-side WaitAsync(2 s), so the red is a failure and not a hang — and
that exception swap IS the finding restated as an assertion.
Host.Tests 137/137.
The default decider restarts a faulting child immediately and without limit, and
the hook makes every restart do ShutdownAsync+InitializeAsync+SubscribeAsync, so a
driver faulting on every value became a reconnect storm. Per-driver last-reprovision
stamp + DefaultReprovisionBackoff (10s, matched to DefaultReconnectInterval);
a held request is re-delivered by a single per-driver timer carrying the original
incarnation ref, so the F2 identity check still applies.
Seen RED with the backoff disabled: InitializeCount == 3 (second applied
immediately) where the test requires 2-then-3.
Also: the _children.Keys union-arm test (seeder gained taglessDrivers) and the
_desiredByDriver xmldoc note that it lingers on last-known-good after a failed apply.
A ReprovisionRequested in flight from an old incarnation could reach the NEW child
after HandleRestartDriver swapped it in. In Connecting that child DOES handle
InitializeRequested, so it started a second concurrent InitializeAsync, racing the
#527 dispose-before-overwrite.
Seen RED: InitializeCount == 2 on a child held in Connecting by a gated init
(an already-connected child swallows InitializeRequested and passes vacuously).
The alarm subscribe gated on 'a handle exists', which is correct for an idempotent
re-push and wrong for a deploy that widens/narrows the set. Combined with the
spawn-time desired set, a respawning deploy could latch the PREVIOUS revision's
alarm refs for the life of the connection; sourceNodeIds-filtering drivers
(OpcUaClient/AbCip/Focas) would never deliver the new alarm.
Seen RED: LastSubscribedAlarmRefs stayed [alarmX] after a deploy widening to
[alarmX, alarmY].
Seen RED against a T3-reverted build: all 4 failed, Created[1].SubscribeCount == 0
on the headline case. 20x isolation run green after the fix was restored.
MapZbLocalDbSync returns IEndpointRouteBuilder, not an IEndpointConventionBuilder,
so .AllowAnonymous() cannot be chained onto it the way the other four mesh maps
are (T2). app.MapGroup(string.Empty).AllowAnonymous().MapZbLocalDbSync() carries
the same convention while leaving the gRPC route pattern byte-identical -
verified directly (LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical).
The census's RealMethodEndpoints filter had to move off a "route pattern starts
with /" heuristic to an exact-route-pattern-set match: inside the empty-prefix
group, grpc-dotnet's own "Unimplemented service"/"Unimplemented method" fallback
endpoints also gain a leading "/", so the heuristic let them back in and
over-counted (caught by DriverOnly_WithASyncPort_AlsoMapsTheLocalDbSyncEndpoint
and LocalDbSync_AllowAnonymousGroup_LeavesTheRoutePatternByteIdentical going red
against the naive filter before this fix).
All five mesh gRPC endpoints now carry IAllowAnonymous (archreview 03/S14, T3).
.AllowAnonymous() on DeploymentArtifactGrpcService, CentralCommandGrpcService,
TelemetryStreamGrpcService and NodeCommandGrpcService — the four MapGrpcService
maps that are chainable. On a login-enabled admin node the global cookie
FallbackPolicy (RequireAuthenticatedUser) applies to every endpoint without
explicit authorization metadata, gRPC included; the PSK bearer interceptors run
INSIDE the endpoint invocation, strictly after that middleware, so a
correctly-keyed node call was challenged before the interceptor's verdict could
matter. Safe because all four interceptors fail closed on an unset key.
The census now asserts IAllowAnonymous on all four (red before this change,
green after) plus a scope guard proving the opt-out never leaks onto an
unrelated endpoint. MapZbLocalDbSync (T3) is not chainable the same way and is
asserted absent here on purpose, so this test cannot start passing for the
wrong reason once T3 lands.
Behaviour-preserving: Program.cs:743-784 collapses to a single call, same gates,
same order, every existing per-map rationale comment carried over. A new
endpoint-metadata census (MeshGrpcAnonymousMetadataTests) drives the SAME
production mapper across all three role shapes (admin-only / driver-only /
fused), so the guard can never assert a shape production no longer has -
archreview 03/S14, T1.
Joseph Doherty