GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-28 22:55:49 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,873 Commits 77 Branches 105 Tags
master
Commit Graph
6 Commits
Author SHA1 Message Date
Marc Schier 2836da7ec8 Fix sessionless ResultTransfer ownership bypass and other issues. (#4834) 
# Description

Correct ownership, endpoint validation, policy propagation, subscription
delivery, file handling and generated-model behavior across the stack,
tools and samples. Add focused regression tests and update the
corresponding guides and sample documentation.

### Previously included corrections

- Require an active, matching session to generate or access transient
ResultTransfer file handles. Cover both sessionless generation and reads
against a handle owned by an active session, using live session owners
in the Machinery tests.
- Keep physical filesystem operations within their configured mount and
apply the same confinement checks throughout file operations.
- Validate LDS semaphore paths consistently during server registration
and registration-store maintenance.

### Individual issues addressed

| # | Component | Issue and correction |
|---|---|---|
| 1 | AI jobs and transfers | Jobs and transfers did not consistently
retain their creator's identity and session. Apply ownership checks to
descendants, file handles, subscriptions and execution, and recheck
deployment permissions before dispatch. |
| 2 | Password-management client | Password-bearing calls could use
signing-only channels or follow a replacement connection. Capture an
encrypted transport generation and keep dispatch bound to it; preserve
ordinary non-password edits. |
| 3 | GDS and certificate-push clients | Connections omitted hostname
verification. Validate the original configured DNS name or IP address
before session activation. |
| 4 | Certificate push | Private-key uploads could use signing-only
calls. Require an encrypted, generation-bound dispatch while preserving
certificate-only updates and restoring the caller's identity afterward.
|
| 5 | MQTT client | The .NET Framework custom-trust callback could
discard TLS hostname, certificate-availability and usage errors. Retain
those checks while supporting configured issuer trust. |
| 6 | DTLS handshake | Certificate trust was not tied to the configured
peer address. Match the certificate's DNS or IP identity to the intended
endpoint. |
| 7 | Replicated address spaces | Serialized nodes lost per-node and
descendant permission metadata. Preserve role permissions, user-role
permissions and access restrictions, including updates to existing
replica nodes. |
| 8 | MCP PubSub imports | Imports without groups skipped checks on the
effective broker destination. Check the actual endpoint before resolving
credentials or activating the connection. |
| 9 | Reference-server accounts | A disabled username account could
authenticate again with its previous roles. Reject disabled accounts
during fresh authentication. |
| 10 | Console reference client | The sample disabled hostname checks
when connecting to an otherwise trusted peer. Retain the original
requested host and validate it before activation. |
| 11 | Capture replay and export | Replay and raw export could accept
non-capture files or include key material. Require complete PCAP/PCAPNG
containers and exclude key sidecars and decryption-secret blocks. |
| 12 | PubSub adapter fallback | The fallback client configuration
automatically accepted peer certificates. Apply normal trust and
hostname validation before activation or reads. |
| 13 | Kafka transport | Explicit TLS enablement was checked but did not
reach both final Confluent configurations. Use the same effective
protocol for validation, producers and consumers. |
| 14 | PubSub actions | Configuration rebuilds could widen a
connection-local action handler's scope. Preserve its original
connection association and reject mismatched dispatch. |
| 15 | PubSub readers | Reader-level message-protection settings were
accepted but ignored. Apply the effective group and reader settings
before accepting and publishing received values. |
| 16 | PubSub key groups | Different `SecurityGroupId` values could
share one connection-wide key context. Reject mixed groups on a
connection and require separate connections for distinct groups. |
| 17 | MCP session registrations | Session-name replacement or reuse
could retain another registration's credential bindings. Bind access to
the registration, caller and authenticated endpoint, and preserve that
identity when borrowing sessions. |
| 18 | Viewer HTTP host | Requests reached the viewer's OPC UA session
without consistent Host, Origin and port checks. Validate the request
destination and browser origin before using the session, while retaining
supported local MCP clients. |
| 19 | Vision file operations | File operations did not consistently
apply protected-file checks, and remote handles lacked caller and
connection context. Apply the shared checks and use host-side opaque
handles bound to caller, target and generation. |
| 20 | DTLS record integrity | The integrity-only MAC used a truncated
sequence number. Authenticate the full reconstructed sequence number and
epoch so records remain distinct across rollover. |
| 21 | AMB client sample | Connection setup omitted hostname validation.
Preserve the originally requested host and verify the peer identity
before activation. |
| 22 | Robot sample writes | The R2 `SpeedOverride` callback could
target R1 state. Bind each callback to its own robot and apply that
robot's write policy. |
| 23 | AI result properties | Optional result children inherited
remotely writable defaults. Preserve their declaration restrictions and
keep server-produced results read-only. |
| 24 | WSS peer-trust updates | Existing channels did not consistently
respond to peer-trust changes. Revalidate complete chains for forward
and reverse WSS channels and close only those that no longer validate. |
| 25 | GDS revocation checks | Caller-provided certificates could cause
issuer retrieval from embedded URLs. Resolve validation from configured
local trust, issuers and CRLs without fetching caller-provided
locations. |
| 26 | GMS observations | Published correction observations retained
writable model defaults. Reject remote writes while preserving
publisher-side updates. |
| 27 | Production-dataset monitoring | Subscription admission and
delivery could use the reporter's context instead of each subscriber's
permissions. Evaluate the subscriber at admission and on every delivery,
including cancellation and disposal paths. |
| 28 | WoT MQTT destinations | Hostless MQTT forms could imply a
localhost destination before loopback checks. Reject them before
credential lookup or connection attempts and validate the effective
endpoint. |
| 29 | XRegistry opaque reads | Content-ID reads and notifications did
not consistently honor encrypted-read requirements or the currently
accepted transport. Recheck the bound endpoint and reject transport
downgrades before rebinding, including when protected values are already
queued. |
| 30 | PubSub capture dissection | Key loading did not consistently
check the configured capture-root boundary. Check each actual key path
immediately before opening it. |
| 31 | PubSub publication queues | Imported WriterGroup queues were not
fully checked against publish-topic grants. Check the destination
actually used by each group. |
| 32 | MCP PubSub mutations | A caller-supplied mode could select
subscribe grants for an operation that publishes. Derive the required
direction and parent context from the actual mutation. |
| 33 | Generated write masks | Model import and generation could widen
an explicit `UserWriteMask`. Preserve it through import, copy, merge,
inheritance and dependency metadata. |
| 34 | Generated method flags | An explicit `UserExecutable=false` could
be lost. Preserve its specified state and resulting method behavior
through the same model paths. |
| 35 | Generated role identifiers | Unresolved role identifiers could
cause permission entries to be omitted. Preserve namespace-qualified
role IDs and report a diagnostic when a policy cannot be represented. |
| 36 | Aggregation-server defaults | The `localhost` default did not
reliably constrain listener binding. Use numeric loopback for the
default endpoint and command-line fallback. |
| 37 | Certificate-validation cache | Cached results could skip current
leaf or issuer validity checks. Recheck validity windows on cache hits
and keep hostname validation independent of cached chain trust. |
| 38 | Software-update folders | Revision names and marker contents
containing dot segments could select paths outside the software root.
Normalize and check every derived path against that root. |
| 39 | USDA arrays | Array elements were not encoded for their string or
asset context. Escape them as data and preserve valid composition and
round-trip behavior. |
| 40 | Adapter hot reload | A changed dataset definition with the same
name could keep old fields, subscriptions or cached values active.
Compare complete immutable definitions and retire old state before
replacement. |
| 41 | Adapter index ranges | Subscription publication could ignore
`IndexRange` and return the complete value. Preserve the requested
projection and its status and timestamps; reject unsupported range
providers. |
| 42 | PubSub metadata and targets | Metadata from one connection could
change another connection's target mapping. Scope metadata to its
connection and provenance, and resolve targets using actual typed field
IDs rather than field names or unintended positional fallback. |
| 43 | Scales product locks | Product locks were not applied across
every remote mutation path. Apply them consistently to value writes,
filling calculations and related recipe/controller operations while
retaining permitted updates. |
| 44 | Event permission caching | Cached event decisions could outlive
identity, role or node-policy changes. Revalidate the current
subscriber, event type and source permissions instead of reusing a stale
decision. |
| 45 | WoT Modbus destinations | Hostless Modbus forms could imply
localhost before loopback checks. Reject them before connecting and
validate the effective endpoint. |
| 46 | MCP local PKI tools | Peer-trust mutations did not consistently
require the caller's `CertificateAdministration` grant. Check the
capability before trust and untrust operations. |
| 47 | Robot-control HTTP bridge | The bridge lacked consistent Host,
Origin and port validation and caller-specific session grants. Apply
those checks before borrowing the OPC UA session and retain supported
local clients. |
| 48 | Subscription transfers | Transferred subscriptions could retain
cached notifications after the destination's permissions changed.
Require authorization continuity between source and destination
identities and roles, invalidate stale retransmission data when
continuity fails, and preserve retransmission behavior for compatible
transfers. |

### Compatibility notes

- Replication uses the versioned NSRP v1 policy envelope. Upgrade peers
together and re-encode or reseed legacy topology from the authoritative
model; missing permission metadata is not inferred from old records.
- Integrity-only DTLS peers must be upgraded together for the full
sequence/epoch MAC. AEAD behavior is unchanged.
- Distinct PubSub key groups require separate connections.
- Custom sessions that cannot capture the actual encrypted transport,
and subscription providers that cannot honor a requested range, are
rejected rather than silently changing the requested behavior.
- Subscription transfers retain retransmission data only when the
destination authorization remains compatible with the source snapshot;
otherwise the old sequence numbers and cached data are invalidated.

## Related Issues

No separate tracking issue is linked. The individual corrections are
listed above.

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
 2026-10-21 05:37:29 +02:00
Marc SchierperfCopilot
 		a10f9e252d Generic runtime continuation, batch lifecycle and readiness (#4759) 
# Description

This pull request extracts and integrates generic server runtime and
continuation-management capabilities for OPC UA NodeManagers.

- **NodeManager lifecycle and routing:** add batch registration and
reload, staged publication and readiness notifications, captured routing
snapshots and reservations, and safe retirement. Wire the lifecycle
through host interfaces, async/sync adapters, `MasterNodeManager`, and
`StandardServer` startup and shutdown.
- **Browse and History continuations:** retain continuation ownership
and manager dependencies across requests and reloads; add atomic claims,
session caching, and cleanup when requests fail or generations retire.
Add historian resume state, tokens, dependency capabilities, and shared
continuation-store support.
- **MonitoredItem lifecycle:** support detaching and reattaching items
across node-manager generations, including event subscriptions shared
across root notifiers and correct defaults for detached-item
modification.
- **Runtime NodeSet references:** track reference additions and
deletions through supported API paths and preserve the tracked
references during reload.
- **Published views and materialization:** add immutable `TypeTable` and
`EncodeableFactory` snapshots with publication-owner lifetime
management, and provide a live node-materialization query for the
existing WoT projection-view consumer.
- **CI trace support and documentation:** add scripts and tests for
staging NUnit internal traces, and document NodeManager lifecycle,
session continuation ownership, and historical-access behavior.

## Related Issues

- Extracts the generic runtime portion of #4511.
- #4757 depends on this pull request; this pull request does not depend
on #4757.

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [ ] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: perf <perf@local>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-10-19 16:08:37 +02:00
romanettandClaude Opus 5.5 9b034f8291 Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700) 
# Description

Nullable reference types are now enabled once, in `common.props`, and
every project in the repository builds warning-clean with them.

**Configuration**
- `<Nullable>enable</Nullable>` lives in `common.props`. All per-project
`<Nullable>` settings are removed: 178 project files (enable,
annotations, and the `disable` in the three Aot test projects).
- 419 per-file `#nullable enable`/`restore` directives and the 4
`#nullable disable` directives are removed.
- Kept on purpose:
- The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type
Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`,
`PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the
project setting in generated code, so the directive is required there.
- `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against
the 1.5.378 packages and does not import the repository props.
- `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now
describe the central setting.

**Migration** (the 43 unset and 13 annotations-only projects, roughly
10k diagnostics)
- **Production code: annotated at the declarations.** Covers
`tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the
LDS sample.
- Members, parameters and returns are nullable where null actually
flows.
- Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`,
`IsMethodTypeDesign`).
- `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take
`params object?[]`. `Template.AddReplacement` takes `string?` (rendering
already skips null replacements).
- `IRegisteredServerStore.RegisterAsync` returns
`Task<RegistrationEntry?>`: it returns null when a registration is
removed.
- Dereferences backed by an invariant (for example, the validated model)
use `!`, so runtime behaviour is unchanged.
- **Behavioural changes in production code** (small, deliberate):
- `NodeSetToModelDesign` throws `InvalidOperationException` when the
file is not a NodeSet. Before, it threw a NullReferenceException on the
next line.
- `ModelDesignValidator` starts its lookup tables empty instead of null.
A redundant `if (m_nodes == null) return;` guard is gone.
- **Test code: minimal annotations.**
  - `?` on values that can be null, `!` where the test asserts non-null.
  - `= null!` for fixture members assigned in SetUp.
- `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the
`IEncodeable` constraint.
- A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`)
folded into the adjacent `IsNull`/`Count` checks.

**Notes for reviewers**
- The diff is large (~1,500 files) but almost entirely mechanical: `?`,
`!` and directive removal. The non-annotation changes are listed above.
- Known follow-up, not done here:
`IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay
non-nullable even though the validator returns null before it loads a
dictionary. Making them nullable would ripple through every generator
call site.

## Related Issues

- Fixes #4439

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

The "no new warnings" box covers net10.0 and net48: full non-incremental
builds of `UA.slnx`, `tools/SourceGeneration.slnx`,
`tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0
warnings and 0 errors under the repository's warnings-as-errors
settings. net9.0 and net8.0 also had no nullable diagnostics, but they
were not rebuilt after the final fixes. Tests have not been run locally
yet.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 08:48:13 +02:00
Marc Schier fa8d45f2cf Fix server/core validation, lifecycle and protocol regressions (#4477) 
# Description

**Server/core audit fixes with regression tests. Latest review-fix
validation, merge validation and earlier acceptance results are
distinguished below.**

This addresses a 95-finding server/core report. Every finding was
reviewed with
the OPC UA specification knowledge base or, where the standard does not
prescribe a C# implementation, the corresponding ownership/configuration
contract. Incorrect or withdrawn subclaims remain qualified rather than
being
presented as additional fixes.

The original checkpoint covers session identity/certificate validation
and
revocation; certificate/key ownership and transactional updates; TCP
reconnect,
receive/admission and buffer lifetime; HTTP quotas; subscription
admission,
durability and pooled payloads; file-operation lifecycle; historian
aggregates;
fluent/state-machine behavior; and related utilities. It includes
targeted
regressions and directly related documentation.

The approved acceptance cleanup is separate in the commit history:

| Cleanup | Verified result |
| --- | --- |
| Invalid identity-policy and KeyCredential fixture inputs | Fixtures
explicitly advertise Username and use its generated policy ID.
Credential persistence uses valid clear mode inside the existing
encrypted admin context. Original permission, lockout and negative
encryption assertions remain. |
| Native GDS lifetime and concurrent connection failures | Retain
ApplicationInstance until client teardown. A deterministic native
reproducer proved the test-only channel cap of 10 was exhausted;
removing that override restores the existing bounded default of 1000. No
global serialization or production quota change. |
| Stress certificate ownership and close accounting | Balance two helper
references and the fake transport's server-certificate ownership.
Replace the invalid event-count bound with exactly-once physical
disposal, opens = closes, zero active entries and one logical entry
lifetime. |
| Participant deadlines | Use the injected TimeProvider with the
existing cross-framework WaitAsync overload. Tests pin one tick before,
exact expiry, terminal notification, recovery and timely completion. |
| Reproducibility and platform assertions | Preserve canonical WoT
bytes/hashes, explicit display-culture contracts and exact
numeric-vector assertions. Accept Apple's CryptographicException subtype
without weakening disposal balance. |
| Missing user-signature proof | Reject null/empty proofs before OS
crypto. Return BadUserSignatureInvalid per Part 4, 5.7.3.3. None-policy
and valid-signature controls remain. A CI-only test nullable-context
error was corrected without changing production behavior. |
| Durable sample restore and shared test storage | Dequeue waits for
actual batch residency instead of treating failed restore completion as
success, and restored files close before deletion. Five deterministic
regressions failed before the fix. The two fixtures that clear the
shared batch directory are isolated; all original 3,000-item assertions
remain unchanged. |

## Latest review follow-up

Commit `828856850219518970e8e2f580b8f94b3c1fc748` fixes the deferred
robotics
disposal reported in the latest review.
`RobotIntentNodeManager.DisposeAsync`
now joins a shared controller-host drain and then explicitly awaits base
node-manager disposal. A controller's bounded shutdown wait may defer
its
cleanup, but no longer lets the node-manager override finish early.
Concurrent
callers, including an asynchronous call after synchronous `Dispose`,
cannot
bypass the drain after the host list has been detached.

Synchronous disposal remains nonblocking for deferred work. Deferred
initialization failures are logged and retained, and base cleanup
failures
propagate to asynchronous callers. The existing controller-level bounded
disposal contract is unchanged; the node-manager contract is documented
in
`docs/Robotics.md`.

| Regression requirement | Evidence |
| --- | --- |
| Wait for deferred hosts; preserve synchronous compatibility and
concurrent joins | `NodeManagerAsyncDisposalWaitsForDeferredHostsAsync`:
four cases covering both entry-point orders and zero/positive host
shutdown waits. All four failed before the fix. |
| Wait through base cleanup and observe failures |
`NodeManagerDisposalWaitsForBaseCleanupAndReportsItsFailureAsync`: four
cases hold the actual base-resource semaphore, verify pending callers
after the host finishes, then check success or the same propagated
exception and exactly-once cleanup. |

The focused `IntentHostingCoverageTests` and
`IntentControllerHostTests.Dispose`
selection passed **30/30 on net10.0 and 30/30 on net48**, with no
failures or
skips. The final net10 solution build passed with zero errors and only
the four
previously documented CA2249 warnings in unchanged fuzz-test code. No
new
warnings, analyzer suppressions, dependencies, or weakened assertions
were added.
The review thread received a commit-specific reply and was explicitly
resolved.

This verification is scoped to the robotics feedback; the earlier full
Subscriptions-run limitation remains recorded in the merge section
below.


## Documentation review follow-up (2026-09-14)

Commit `f9a9686fcc5d3cfd19cbbc8c2d826e20f5c7e72f` addresses the six
follow-up
threads on documentation and implementation rationale. Each has an
in-thread
reply citing the commit and an explicit resolution.

Multiline XML documentation now covers the declarations introduced or
modified
throughout the entire PR, including internal contracts, samples, tests,
and
public members of related private helpers. A syntax-aware inventory
checked
**2,031 declarations across all 256 PR C# paths with zero documentation
gaps**.
A supplementary check also covers **75 previously unchanged public
members**
of private helpers involved in the changed code. Token comparison across
the
conditional-compilation branches proves this follow-up changes **no
executable
C# or test assertions**. Project-file changes are rationale comments
only.

The implementation questions are documented at their source:

| Question | Resolution |
| --- | --- |
| Certificate assembly friendship | Server Push configuration reuses the
internal explicit-subject CSR helper with the existing key; no public
crypto API was added. |
| Core.Types assembly friendship | Server builders use internal
transition callbacks/revision guards; Server.Tests checks the revision
deterministically without creating a public compatibility contract. |
| Always-single-reader notifications | One dequeuer preserves event
order; optional concurrency fans out within one event and joins before
the next event. |
| Asynchronous disposal completion | `DisposeAsync` already joins the
completion published after admitted operations and all cleanup attempts.
The compatibility `Dispose` remains nonblocking to avoid deadlocking an
admitted callback against its own drain. |

The requested detailed review disposition follows. It distinguishes
actual
fixes from confirmed policies and incorrect subclaims rather than
counting every
comment as a new runtime change.


### Detailed disposition of review 5191795284

The [SDK
review](https://github.com/OPCFoundation/UA-.NETStandard/pull/4477#pullrequestreview-5191795284)
was submitted against `1c4a9df6c`, before the subsequent merge and
feedback fixes.
Its **15 inline threads** have each received an action-specific reply
and an
explicit resolution. A fresh GraphQL query verified those replies and
resolution
states; the fixes in `b8a59990f` remain ancestors of the current branch.

| Review concern | Action and disposition | Concrete regression evidence
|
| --- | --- | --- |
| Injected certificate-store providers bypassed | **Fixed.**
Registration, configuration snapshots, and validation use the injected
providers. Explicit custom types remain authoritative; the built-in
Directory catch-all is deferred so native X509 paths and later custom
providers are not captured. Failed store opens dispose the created
store. | `InjectedProviderResolvesRegisteredAndConfiguredTrustStores`
covers default-provider ordering and both registration paths; provider
validation/open-failure cases and the unchanged
`TrustedCertificateInjectedViaIndependentX509StoreIsObservedAsync` cover
real use and independent-store visibility. |
| Unguarded timer-thread leadership callbacks | **Fixed.** Each
subscriber's exception is logged independently and cannot suppress
subsequent subscribers or fail lease operations. `IsLeader` reads the
deadline without invoking application callbacks. |
`ThrowingLeadershipSubscriberDoesNotFailAcquireOrDisposeAsync`,
`ExpiryNotifiesOtherSubscribersWhenOneThrowsAsync`, and
`IsLeaderDoesNotDispatchExpiryNotificationsAsync` were all demonstrated
failing before the fix. |
| Legacy pending-key provider accepts an unfinishable regenerated CSR |
**Fixed by explicit capability validation.** A provider implementing
only `IPendingCertificateKeyStore` receives `BadNotSupported` before key
generation or persistence when regeneration is requested. Existing-key
CSR/update remains supported. Regeneration requires matching-only
consumption; no unsafe destructive probing was introduced. |
`LegacyPendingProviderRejectsRegenerationBeforeCreatingOrSavingAKeyAsync`
checks no key or store mutation;
`LegacyPendingProviderCompletesExistingKeySigningAndUpdateAsync` proves
the supported flow completes. |
| Shutdown waits behind user deactivation | **Fixed.** Accepted
deactivations drain before the master's lifecycle gate is taken.
Configuration retains its binding until the shared drain completes,
including concurrent shutdown callers and direct master disposal. | Four
cases of `ShutdownDrainsDeactivationOutsideTheSessionClosingGateAsync`
exercise real configuration/binding/ServerInternalData/master reentry,
retained address space, exact session/subscription closure, and no
production cancellation shortcut. |
| Subclass cleanup runs under the operation-lifetime lock | **Fixed.**
Cleanup starts outside that lock. A pre-published completion joins all
disposers, and only the active asynchronous teardown context may use
guarded helpers. Captured contexts lose that privilege when the callback
exits. |
`SynchronousOwnedCleanupLetsSamplingWorkerReachClosedAdmissionAsync`,
`DisposeCoreCanUseGuardedMembersWithoutReopeningCallerAdmissionAsync`,
captured-context rejection, and exact cleanup-failure/ownership
assertions cover both synchronous and suspended cleanup. |
| Discovery requestId-zero chunk read after pool return | **Fixed by the
earlier merge.** Discovery validation precedes `SaveIntermediateChunk`;
no read occurs after that path can return the buffer. The ordering is
retained. | `DiscoveryChannelReadsTheFirstChunkBeforeItIsReturnedAsync`
uses a poisoning pool and the requestId-zero path. |
| T1.8 OPN cleanup misses post-decryption throws | **Fixed by the
earlier merge and pinned with new regressions.** Both sequence rejection
and certificate mismatch release the decrypted body and input ownership.
| `OpenSecureChannelRejectionAfterDecryptionReturnsEveryRentalAsync`
checks the exact audit status and pool conservation. Omitting either
corresponding release in an isolated snapshot made its case fail; no
mutation remains. |
| T1.2 mount-root protection depends on the provider | **Fixed at the
host boundary.** Separator-only aliases resolve to the protected root.
Traversal, empty/whitespace segments, backslashes, and drive/stream
qualifiers are rejected before arbitrary providers are invoked. Valid
nested paths and root destinations remain supported. | A deliberately
permissive provider is used by
`NonCanonicalObjectPathsCannotDeleteMoveOrCopyAsync` and invalid-target
cases; exact zero-call checks are paired with legitimate
nested-path/root-destination controls. |
| Inline trusted/issuer entries now face CRL checks | **Intentional
strict behavior, documented and tested.** A configured backing store
with strict unknown-status rejection requires current revocation
information. Explicit-only lists without a store retain their no-store
behavior. Actual revocation cannot be suppressed; a matching store
entry's policy takes precedence. |
`InlineIssuerWithStoreHonorsStrictUnknownRevocationPolicyAsync`,
`InlineIssuerRevocationIsNeverSuppressedAsync`, store-precedence cases,
and explicit-only controls preserve the missing-information versus
actual-revocation distinction in Part 4 6.1.3. |
| RSA minimum applied to unspecified ECC identifiers | **Fixed at the
application-certificate check.** The RSA minimum is compared only with
an actual RSA public key. An unspecified ECC identity is not replaced
simply because its key size is below an RSA threshold. |
`SilentCertificateCheckPreservesUnspecifiedEccIdentityAsync` checks DER,
thumbprint, store contents, and private-key retention; explicit RSA and
HTTPS minimum controls remain. |
| Failed directory refresh wedges corrective mutations | **Fixed.**
Recoverable automatic pre-refresh failures are logged and leave
reconciliation pending, but do not block corrective provider mutations.
Explicit refresh errors, capacity admission, caller cancellation, and
actual provider denials remain enforced. |
`CorrectiveDeleteRecoversAfterPersistentOverLimitRefreshFailureAsync`
converges an externally over-limit directory back to its exact allowed
contents. Capacity, cancellation/token, and six refresh-error controls
check negative paths. |
| Timed-transition rejection logs at 10 Hz | **Fixed.** Repeated
`(state, revision, status)` warnings are deduplicated without changing
the retry cadence. A different status or state entry permits another
warning. | `RepeatedTimedRejectionLogsOnceAndRecoversAsync` checks one
warning over eleven rejections and success on the next attempt.
Status-change and same/different-state reentry controls pin reset
behavior. |
| Bad unsubscribe strands retired-generation bookkeeping | **Fixed.**
Retired all-event bookkeeping completes even when unsubscribe reports
Bad; the first Bad is still returned to the caller. | Good and
BadCommunicationError cases of
`RetiredAllEventsDeletionCompletesBookkeepingAndReleasesGenerationAsync`
check event/snapshot removal, generation release, and exactly one
unsubscribe without finalization replay. |
| CSR subject parsing and fallback | **Fixed for existing and
regenerated keys.** Omitted subjects fall back to the actual stored
certificate. Regeneration resolves the public certificate's subject
before creating a key. Malformed or unresolvable subjects return
`BadInvalidArgument` before pending-key mutation. | Store-only fallback,
real-generator null/empty cases, and missing-subject/malformed-DN
controls verify the signed CSR, matching fresh pending key, unchanged
active identity, and no destructive side effects. |
| Committed RemoveUser change reported as failed | **Fixed for
RemoveUser and ModifyUser.** Accepted cleanup is still awaited and close
failures remain logged, but cannot replace the committed mutation
result. Precommit failures retain their original result and side
effects. |
`CommittedUserChangeKeepsResultAndPublishedUsersWhenCloseFailsAsync`
checks published Users, Good result, logging, and continued closes.
`FailedUserChangeKeepsPublishedUsersAndDoesNotCloseSessionsAsync` checks
BadNotFound with no publication or close. |
| Alleged pooled decoded-request aliasing | **No runtime redesign
needed; reviewer rebuttal confirmed.** The decoder copies the relevant
input data. Misleading ownership comments were corrected rather than
introducing a new dispatch model. |
`DecodedRequestRetainsValuesAfterInputBuffersAreReturnedAsync` checks
the exact decoded NodeId and ByteString after returned arrays are
poisoned. |

The current documentation-only follow-up rechecked **104 Core, 473
Server, and
4 Configuration cases** covering these concerns: **581 passed, zero
failed or
skipped**. The final net10 solution build also passed with **zero
warnings and
errors**. The complete earlier Core/Server/Configuration and net48
results remain
listed separately rather than being presented as rerun current-head
acceptance.

## Earlier review follow-up

Commit `b8a59990fe2e8e967a12ffd5d9158beed7831143` addresses all 19
review
threads. Each thread has an in-thread reply naming this commit and was
explicitly
resolved after verification; the final paginated query found 19 resolved
and
zero unresolved threads.

The follow-up fixes injected certificate-store selection (including
default
provider ordering and native X509 paths), actual-RSA minimum
enforcement, and
CSR capability/subject validation before key mutation. Node-manager
teardown
uses simpler scoped ownership and drains accepted user revocation
outside the
master gate; committed user results and retired-event cleanup remain
correct
when cleanup reports failure. File mutations can recover after failed
refresh
without bypassing capacity or cancellation, provider paths protect mount
roots,
and repeated timed rejections no longer flood logs. Leadership
subscriber
failures are isolated without callbacks from `IsLeader`.

Regressions also pin the already-merged OPN/discovery buffer fixes and
prove
decoded request values do not borrow returned input buffers. The strict
inline
issuer revocation policy is documented and tested, not weakened.
Existing tests
were changed only for the reviewed behavior: the incorrect
committed-user result
was corrected, and inline-issuer/root-alias controls were expanded.

### Review-fix validation

| Suite | Scope | Framework | Passed | Existing TRX NotExecuted |
| --- | --- | --- | ---: | ---: |
| Core | Complete project | net10.0 | 4,970 | 88 |
| Server | Complete project | net10.0 | 6,414 | 69 |
| Configuration | Complete project | net10.0 | 231 | 0 |
| Core | Certificate, lease and buffer compatibility slice | net48 | 153
| 0 |
| Server | Reviewed lifecycle, user, file, timer and CSR compatibility
slice | net48 | 692 | 0 |
| Configuration | Complete project | net48 | 231 | 0 |
| CSR provider regressions | Final portable-mock follow-up | net10.0 |
11 | 0 |

All runs exited successfully with normal concurrency and leak detection.
The Server console reports 5 skips; its TRX additionally records
existing
assumption/explicit cases, hence the 69-record count above. The
Framework build
caught two modern-only `ValueTask.CompletedTask` usages in new mock
setup; only
those expressions changed to the equivalent portable
`default(ValueTask)`.
The complete modern suites precede that test-only correction; all
production
files remained byte-identical, and the corrected fixture passed on both
targets.

The final net10 solution build succeeded with 0 errors and the same 4
CA2249
warnings in unchanged fuzz-test code. Diff checks and changed-line
whitespace
checks are clean; whole-file formatting still identifies 10 unchanged
baseline
lines, which were not reformatted outside the feedback scope. The
committed
35-file tree was hash-verified against the validation inputs.

This is the review-feedback verification scope, not a rerun or sign-off
of the
historical complete platform/native/soak acceptance campaign below.


## Latest upstream merge

Merge `9d9bf679f317164c5b2b90924cd3cdc27a86d451` integrates `master`
through `162fc5ebb` (#4486),
including the AddNodes child-name index, shared certificate-error
reporting,
CTT sample-data corrections, and the accompanying regressions and
documentation.

The CreateSession conflict preserves this branch's certificate
ownership,
required certificate, application-URI, and channel-certificate checks.
The
validation result now retains nested errors through `ThrowIfInvalid`,
allowing
the upstream shared reporting helper to mask trust, policy, and
revocation
failures consistently without losing audit detail.

The new indexed AddNodes regression initially attempted to reuse a
deterministic
NodeId after renaming its existing node. Renaming frees the BrowseName,
not that
NodeId. `IndexedBrowseNameReusePreservesExistingNodeIdentityAsync` now
proves
both contracts: the derived-ID collision leaves the original node
intact, then
a fresh explicitly requested ID can reuse the old name while the new
name
remains protected against duplicates. No production identity rule was
relaxed.
The incoming seeded random-index test uses the existing `UnsecureRandom`
wrapper
with the same seed and sequence; no new analyzer suppression was added.

| Local merge validation | Framework | Passed | Existing exclusions |
| --- | --- | ---: | ---: |
| Core, complete project | net10.0 | 5,081 | 88 |
| NodeState / BaseInstanceState | net10.0 | 579 | 0 |
| Server node management, certificate and session-security selection |
net10.0 | 112 | 0 |
| InformationModel node-management services | net10.0 | 29 | 1 |
| NodeState / BaseInstanceState | net48 | 578 | 0 |
| Core certificate validation, error reporting and TCP buffers | net48 |
111 | 0 |
| Server node management, certificate and session-security selection |
net48 | 112 | 0 |

All selected runs exited successfully at normal concurrency: **6,602
passing
test executions**. Modified test assemblies were explicitly rebuilt, and
the
combined name/identity regression was verified by name in both net10 and
net48
results. No skips or weakened assertions were introduced.

The final `dotnet build UA.slnx -c Release --no-restore -m:1` with
`CustomTestTarget=net10.0` succeeded with **zero warnings and zero
errors**.
The resolved source and adjusted tests pass whitespace verification, and
their
source hashes match the validation inputs.

This is merge-specific verification. Earlier review results and the
previously
reported incomplete full Subscriptions runs below remain scoped to their
own
commits; they were not reopened or relabeled as passing by this
operation.

## Previous upstream merge (2026-09-15 morning)

Merge `c4694e20ab089c555af233642042133ff3a03ac8` integrates `master`
through `484d6c887`, retaining
#4481's shared Like parser/GDS query fixes, #4488's RequestHandle
recovery in
ServiceFaults, and #4487's expired-session and durable-transfer
corrections.

Three conflict resolutions preserve both sides:

- Alias searches now use the shared `LikePattern` implementation,
including
upstream malformed-list validation. A timeout overload retains this
branch's
100 ms per-alias deadline and `BadTimeout` reporting without keeping a
second
wildcard translator. The shared matcher's original unlimited overload
remains
unchanged for existing GDS/ContentFilter callers. New regressions cover
bounded prefix/list scans, reuse after timeout, and the alias service
result.
- Expired activation closes outside the session-manager gate using
upstream's
once-only timeout claim, audit, and diagnostic accounting. This branch's
  original-channel and security-policy checks remain in place.
- TCP buffer tests retain this branch's ownership, poisoning-pool and
post-decryption checks alongside the upstream RequestHandle/fault
overload
  tests; neither set was discarded.

The upstream HTTPS binary behavior is intentionally retained: a decoder
`ServiceResultException` produces HTTP 200 with an OPC UA ServiceFault
carrying
the decoder status, response timestamp, and recoverable RequestHandle,
rather
than the previous HTTP 500 text response. Other exceptions retain their
existing HTTP error handling.

| Local validation | Framework | Passed | TRX NotExecuted |
| --- | --- | ---: | ---: |
| Core, complete project | net10.0 | 5,054 | 88 |
| Sessions, complete project | net10.0 | 780 | 610 |
| Server aliases, expiry/security and subscriptions | net10.0 | 288 | 0
|
| GDS database, paging and live directory calls | net10.0 | 263 | 0 |
| Shared Like patterns and bounded evaluation | net10.0 | 109 | 0 |
| Subscriptions transfer/durability/service selection | net10.0 | 23 | 1
|
| HTTPS WebApi, complete project | net10.0 | 346 | 0 |
| InformationModel aliases | net10.0 | 54 | 5 |
| Shared Like patterns and bounded evaluation | net48 | 109 | 0 |
| Core fault handles, TCP/HTTPS and filters | net48 | 268 | 0 |
| Server aliases, expiry/security and subscriptions | net48 | 288 | 0 |

These selected runs exited successfully with normal test concurrency:
**7,582 passing test executions**. Sessions' console reports 379 skips;
the
TRX also contains conditional/assumption exclusions, so the table uses
the
actual 610 NotExecuted records. No skips or weakened assertions were
added.
The new `EvaluationTimeoutBoundsWorkAndPreservesPatternReuse` and
`SharedPatternTimeoutRemainsAnAliasServiceError` tests passed on both
targets.

**Validation limitation:** two unfiltered Subscriptions runs did not
produce
a final result and were stopped, including a retry with a hang guard.
They
are not counted as passing. The isolated failover fixture passed 2/2,
and the
transfer/durability/service selection above passed. The stall's cause
was not
established; no unrelated code or test was changed to conceal it.

The final `dotnet build UA.slnx -c Release --no-restore -m:1` with
`CustomTestTarget=net10.0` succeeded with **0 warnings and 0 errors**.
Whitespace verification passed for all hand-reconciled files, and staged
source hashes match the validation inputs.

These are merge-specific checks, not a new full platform/native/soak
acceptance
sign-off. Earlier review and acceptance evidence below remains dated to
its
own commit.

## Previous upstream merge (2026-09-14)

Merge `c3e09553e4d1509502d4ba751960d58491bea2cf` integrates `master`
through `6313e38a0` (#4478),
including the CTT-driven Part 13 aggregate corrections and FindAlias
argument
validation. The upstream tests and CTT documentation are retained.

The single conflict in `AliasNameWildcardMatcher` is reconciled through
the
branch's shared regex compiler: upstream malformed-pattern rejection and
`IsValidPattern` coexist with OPC UA character escaping,
whole-string/newline
matching, the 100 ms timeout, and service-level error reporting. The
upstream
ReferenceTypeFilter checks and chronological aggregate behavior are
preserved.

Local merge verification:

| Suite / selection | Framework | Passed | Existing skips |
| --- | --- | ---: | ---: |
| Server aliases, aggregates and historian | net10.0 | 1,011 | 0 |
| Server aliases, aggregates and historian | net48 | 1,011 | 0 |
| InformationModel aliases | net10.0 | 54 | 5 |
| Features aliases | net10.0 | 11 | 0 |
| Client aliases | net10.0 | 28 | 0 |
| History aggregate/historical-access consumers | net10.0 | 265 | 0 |

All selected tests passed at normal concurrency. The final net10
solution
build passed with 0 errors and 0 warnings.
The resolved file's whitespace check and the staged diff check passed.
These
are merge-specific checks, not a new full platform/native/soak
acceptance claim.

## Previous upstream merges

Integrates `master` through `8e874d073`, including the Client, Types,
Core and
source-generator audit fixes in #4471, #4473, #4474 and #4475, plus the
event queue-size revision from #4480. The merge commits are `150ea0439`
(26 conflict resolutions) and `a0e17c07c` (the base update that arrived
during
validation).

The overlapping fixes are reconciled rather than replacing either
branch:

- **Rejected-certificate retention follows this branch, as explicitly
requested:**
zero means unlimited history; negative values disable new storage while
preserving existing backend trimming behavior (Directory clears prior
history;
shared key/value leaves it intact). The processor and manager policy are
not
  redefined, and documentation/tests describe the distinction.
- Preserve owned certificate snapshots and borrowed validation-core
lifetimes
  alongside upstream cache invalidation, store-first CRL checks and
  `BadCertificateRevocationUnknown` for unsupported revocation lookup.
- Combine exactly-once chunk ownership, receive-loop cancellation,
stale-epoch
rejection and detach-before-adoption with upstream
signature-before-padding
  verification and secret cleanup.
- Preserve pooled `HttpClient` per-request configuration together with
bounded
response reads and deadlines that remain active through body
consumption.
Regression cases exercise already-used clients and both timeout
precedence cases.
- Retain the binary-compatible channel-key constructor and normalize
omitted
profiles; preserve exact profile filtering and upstream binary-first
HTTPS/WSS
  endpoint ordering.
- Preserve monotonic lease expiry and stale-confirmation fencing while
adding
ordered, non-reentrant leadership notifications outside the state lock.

## Validation

Local validation of the conflict-resolution merge:

| Suite | Framework | Passed | Existing skips |
| --- | --- | ---: | ---: |
| Core | net10.0 | 4,940 | 88 |
| Client | net10.0 | 2,278 | 0 |
| Server | net10.0 | 6,088 | 5 |
| Types | net10.0 | 10,394 | 1 |
| SourceGeneration.Core | net10.0 | 3,940 | 8 |
| SourceGeneration | net10.0 | 185 | 0 |
| Core | net48 | 4,855 | 104 |

After the subsequent four-file #4480 update, the related Server slice
passed
811 tests with 5 existing skips, the real durable queue-size cases
passed 6/6,
and EventManager's 21 cases passed again after analyzer-only
expected-array
cleanup. The final `dotnet build UA.slnx -c Release` with
`CustomTestTarget=net10.0`
succeeded: 0 errors and 4 pre-existing CA2249 warnings in unchanged
fuzz-test
code. Whitespace verification passed for the resolved files and the
follow-up.

Merge-specific proof includes
`RequestTimeoutRemainsActiveWhileReadingTheOpenApiBodyAsync` on
already-used
shared clients with either the request or HttpClient timeout shorter,
the
existing ownership/expiry regressions from both branches, and both
branches'
retention tests adjusted only to the explicitly chosen zero/negative
contract.

Earlier acceptance at `1c4a9df6c` passed hosted Linux/macOS CI, CodeQL,
image
builds and the canonical coverage gate (87.15% line, 77.27% branch,
92.14% patch).
Both Framework project matrices, all three Windows native suites (183
cases),
and the unfiltered net10 stress project (230 cases, including full
60-minute
chaos and 30-minute memory soaks) also passed. Those are **pre-merge
results**,
not a claim that the full framework/platform/native/soak matrix has been
rerun
on this merge. The previous acceptance workflow remains separate from
this
review-feedback operation.

Security checks, meaningful assertions, leak detection, default native
concurrency, full soak durations and coverage thresholds remain intact.
The user-set ready-for-review state is unchanged. No architectural or
complete
merged-head acceptance sign-off is claimed.

## Related Issues

- Fixes #4476

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [x] I have addressed **all** PR feedback received.
 2026-09-16 04:42:09 +00:00
Marc Schier 275c3800b5 [Server] Add live NodeManager lifecycle support (#4094) 
# Description

Adds supported live add, reload, and remove lifecycle operations for
NodeManager instances on a running server.

The change introduces a DI-backed `INodeManagerLifecycle` service with
direct `StandardServer` access, generation-aware registration handles,
copy-on-write routing, transactional prepare/commit/rollback, runtime
NodeSet convenience APIs, append-only namespace updates, model and
semantic change notifications, additive-compatible complex type priming,
and complete teardown behavior.

Lifecycle publication is coordinated with requests, sessions,
subscriptions, monitored items, continuation points, and shutdown so
retired managers cannot remain reachable or be disposed while still in
use. Compatible monitored items transfer to replacement generations
without a transient bad status. Removed or incompatible Nodes publish
`BadNodeIdUnknown` as required by OPC UA Part 4 section 5.8.4.1, retain
their MonitoredItem identity, and automatically recover when a
compatible Node with the same NodeId returns.

Registration is documented end to end in the new
[docs/NodeManagerRegistration.md](docs/NodeManagerRegistration.md),
which covers compile-time, startup, and runtime registration together
with the guarantees for MonitoredItems, Browse continuation points,
namespace indexes, DataTypes, and change notifications.
## Review feedback addressed

Interface surface reduced, as requested:

- `INodeManagerMonitoredItemTracker` folded into
`ISubscriptionMonitoredItemLifecycle`.
- `INodeManagerMutationCoordinator` and the asynchronous MonitoredItem
recovery entry point folded into `IDynamicNodeManagerHost`.
- `ISessionClosingRegistry` folded into `IServerInternal`, then replaced
by `ISession.IsClosing`.
- `ISubscriptionDeletionRegistry` folded into `ISubscriptionManager`,
then removed entirely.
- `INodeManagerContinuationPointTracker` folded into `ISession`.
- The synchronous recovery entry point kept on its own
`ISyncNodeManagerMonitoredItemRecovery`, because `AddPredefinedNode`
cannot await; it still routes through the asynchronous
`INodeManagerMonitoredItemLifecycle` that `AsyncNodeManagerAdapter`
exposes.
- `IMonitoredItemLifecycle` renamed to `IDetachableMonitoredItem`.

Reach into the Subscription area reduced to the minimum the feature
needs:

- The lifecycle feature itself leaves `SubscriptionManager` almost
untouched: two guards that reject a request from a Session that is
closing, on `CreateSubscription` and on `TransferSubscriptions`, because
it would create state that is about to be torn down. The deletion claim
registry, the session closing gates, the closed-session bookkeeping and
the deletion restore paths introduced by earlier revisions are gone.
Transferring a subscription away from a session that is closing stays
allowed, since that is the recovery path the service exists for; the
race that exposed is fixed by serialising source-queue closure with the
transfer rather than by rejecting it.
- Queueing is disabled again for a MonitoredItem with `QueueSize <= 1`,
as before this PR. Without a queue the Client is served the last sampled
value, so the missing-Node notification simply becomes that value;
losing values is expected for a MonitoredItem without queuing.
- What remains of the notification is one `DataValue` plus one bool in
`DataChangeQueueHandler`, mirroring the existing overflow pair, which
stops it being the value discarded when a configured queue is full.
- `MonitoredItem` keeps its original publish path, its original queue
creation and restore paths, its original triggering behaviour, and no
per-epoch bookkeeping.
- `IStoredMonitoredItem` carries `IsDeleted` and `IsDetached`, so a
restored item reports the state it was stored with rather than one
inferred from the last value's StatusCode.

Other feedback:

- Documented `HostedNodeManagerLifecycle`, `IDynamicNodeManagerHost`,
`NodeManagerRoutingTable`, and the `RequestManager` validation and
request scopes.
- Renamed `ValidateMonitoredItemAsync` to `CanAttachMonitoredItemAsync`
and `RestoreMonitoredItemAsync` to `RecoverMonitoredItemAsync`, so the
lifecycle API no longer collides with the durable Subscription
`RestoreMonitoredItem` path used at server startup.
- Removed the `CustomNodeManager2` dependency from the synchronous
MonitoredItem recovery path.
- `DataChangeQueueHandler` is back to the two constructors 1.5.378
shipped, each taking an optional `TimeProvider`.
- Session closing is a one-way latch, and the Session is removed even
when teardown throws.
- Documented what a custom NodeManager has to implement to take part in
live lifecycle operations, and which NodeManager implements
`INodeManagerReloadParticipant` today.

## Fixes found during review

- The guard that stops a NodeManager lifecycle operation being started
from inside a service handler never fired. It was backed by an
`AsyncLocal` written inside `async ValidateRequestAsync`, which is
invisible to the caller that awaited it, so a lifecycle call from a
NodeManager callback waited for its own request and held the lifecycle
semaphore forever. The mark is now applied by the service dispatcher,
and the drain is bounded so no path can wedge the lifecycle permanently.
- `ValidateRequestAsync` is no longer virtual. An override that rejected
a request after calling `base` left it registered forever; subclasses
now use `OnRequestValidatedAsync`, whose failures complete the request.
- A MonitoredItem deleted and disposed while a reload was handing it to
the replacement NodeManager could be attached anyway. The item now
reserves itself for the hand-over and the transition takes it back if it
was disposed meanwhile.
- The Overflow bit is recorded and applied when the value is published,
as the surrounding design does, rather than written into the value at
enqueue time.
- MonitoredItem cleanup for a Subscription being deleted no longer
passes null entries to `DeleteMonitoredItemsAsync`.
- Browse continuation points of a retired NodeManager are persisted and
disposed outside the Session lock.

## Open review discussions

Three threads are open, replied to and awaiting reviewer decisions:
separating service-call dispatch from NodeManager handling in
`MasterNodeManager` (tracked as #4098), the request-scoping shape in
`StandardServer`, and how much of the async MonitoredItem service
implementation should stay in `Subscription`.

Issue #4102 records a specification ambiguity found while implementing
the missing-Node notification: Part 4 §5.8.4.1 makes it mandatory, but
the protected over-capacity queue entry §5.13.1.5 defines applies to
`EventQueueOverflowEventType` only, so the specification does not say
how it survives a full queue.

## Validation

- Full `Opc.Ua.Server.Tests` on net10.0: 3,768 passed, 5 skipped.
- Lifecycle, MonitoredItem, Subscription and continuation point suites:
185 passed on net10.0 and 75 on net48.
- Server builds complete with zero warnings or errors.
- Two failures are pre-existing and unrelated:
`DurableDataValueQueueVerifyReferenceBatchingAsync` (flaky
`BatchPersistor` file contention, passes in isolation on net10.0 and
net48) and
`ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`
(fails on a clean tree with these changes stashed).

## Related Issues

- Fixes #3993
- Follow-up tracked in #4098
 2026-07-28 16:06:28 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00