GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-21 00:56:28 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,739 Commits 110 Branches 105 Tags
fix/encoder-stack-depth-json-flake
Commit Graph

18 Commits

Author SHA1 Message Date
romanett 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
romanett 0b9af4db34 Fix flaky BuilderDisposesOwnedHttpProviderAndChannelManagerAsync on net48 (#4597) 
## Problem


`ManagedSessionResourceOwnershipTests.BuilderDisposesOwnedHttpProviderAndChannelManagerAsync`
failed on net48 in [run
36598770373](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36598770373)
(job `test-windows-net48 (5/30)`, on PR #4595, which only touches
`version.json`). It failed after 12.1 s with a `TaskCanceledException`.

This was not a disposal problem. The TRX shows that `ConnectAsync` never
finished:

- The shared channel entered `TransportConnecting` and never reached
`Ready`.
- After 10 s the test's connect deadline cancelled
`ConnectionStateMachine.WaitForConnectedAsync`, with
`ClientChannelManager.GetCoreAsync` still waiting on the open.
- The open was still running after the shutdown token was cancelled, so
it was stuck on something that ignores cancellation.
- On passing runs this test takes about 0.4 s on CI.

## Root cause

While opening, `WebApiTransportChannel` sends a sessionless
`GetEndpoints` to fill in the endpoint description. That request goes to
the test's scripted `HttpMessageHandler`. On .NET Framework the handler
read the request body with the tokenless `ReadAsByteArrayAsync()`. For a
`ByteArrayContent` this copies the bytes through `Stream.BeginWrite`,
which runs on a thread-pool thread. On a starved runner the read can
therefore stall indefinitely. Because it cannot be cancelled, it
outlasts both the channel's own 6 s `GetEndpoints` timeout and the
test's 10 s deadline. On net8+ the handler already passes the token.

## Proof

- **Non-cancellable 12 s delay** injected at that point in the handler:
reproduces the CI failure exactly. The channel sits in
`TransportConnecting` for 10 s, then `TaskCanceledException` is thrown
from `WaitForConnectedAsync`.
- **Cancellable 12 s delay** at the same point: the channel's 6 s
`GetEndpoints` timeout cuts it off, the channel opens and the test
passes.
- **Probe on net48:** `ReadAsByteArrayAsync()` on the request content
never completes synchronously, while `ReadAsStreamAsync()` on a
`ByteArrayContent` always does.

The product's own response reading (`HttpResponseBodyReader`) already
uses `ReadAsStreamAsync` and passes the token, so only the test handler
is affected.

## Fix

On .NET Framework the handler now reads the content stream, which
`ByteArrayContent` creates synchronously, and copies it in memory. There
is no thread-pool hop left in the handler.

## Testing

`ManagedSessionResourceOwnershipTests` passed 15 of 15 runs on net48 and
15 of 15 on net10.0. Before the fix, the test also passed 150 repeats
locally, including under full CPU load.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-09-30 08:05:09 +02:00
romanett 87c35b84b6 Fix Opc.Ua.Client audit findings (security fallback, reconnect, subscriptions, redundancy, browse, files, WebApi) (#4572) 
## Summary

Fixes the findings of a bug audit of `Opc.Ua.Client` and
`Opc.Ua.Client.ComplexTypes`, done on master plus #4516 (now merged).

- **Audit:** 82 findings across 12 areas, each checked against the OPC
UA specification. Final severities: 2 High, 37 Medium, 43 Low.
- **Fixes:** 76 fixed, 4 partial, 2 skipped (see [Not
fixed](#not-fixed--partial)).
- **Review:** a follow-up review of the fixes found 15 more defects in
them, all fixed here.
- **Tests:** every fix has a regression test, except a few races that
cannot be reproduced deterministically; the commit messages say which.

The finding IDs (`L<area>-<n>` for the audit, `G<n>` for the review)
appear in the commit titles.

## Behaviour and user-facing changes

### Security and endpoint selection
- **Reconnect no longer drops to weaker security (L3-1, High).** When
the server rejects the configured security policy or mode during
reconnect, `SessionReconnectHandler` no longer switches to whatever
endpoint the server advertises, None included. It only picks an endpoint
at least as strong as the configured one, judged by a locally computed
security level. The server-advertised `SecurityLevel` is not trusted. If
nothing qualifies, reconnect fails with the original error (new log
event `SessionReconnectHandler + 13`) and keeps retrying with the
original settings.
- **`CoreClientUtils.SelectEndpoint(useSecurity: true)` never returns a
SecurityMode None endpoint over opc.tcp (L9-9, High).** It now returns
`null` when no secure endpoint matches.
- When the discovery URL is `https`/`opc.https`, an HTTPS endpoint with
SecurityMode None is still accepted as a last resort, because TLS
protects it (Part 6 §7.4.1) (G17).
  - Endpoints with SecurityMode `Invalid` no longer count as secure.
- `GlobalDiscoveryServerClient` and `ServerPushConfigurationClient` now
throw `BadSecurityPolicyRejected` when no secure endpoint matches,
instead of passing `null` on. The ConsoleReferenceClient sample reports
this with a clear error.
- **No Bearer/Basic credentials over `http://` (L12-2).**
`WebApiClient.Create` and WebApi channel open/reconnect reject them with
`BadSecurityChecksFailed`. The public `WebApiClient(HttpClient, …)`
constructor also rejects them when the client's `BaseAddress` is not
https, and every send checks the address it actually goes to (covers a
`BaseAddress` set later).

### Session, keep-alive and identity
- **Faster detection of a silent server (L1-1).** The keep-alive Read is
now bounded by its `TimeoutHint` (2 × `KeepAliveInterval`), which is
what Part 4 §7.32 says the client should do. It then fails with
`BadNoCommunication`, so reconnect starts after about 2 ×
`KeepAliveInterval` instead of `OperationTimeout` (120 s by default).
- **Operations fail fast during a channel-manager recovery (L1-2, G9).**
`UpdateIdentityAsync`, `TransferSubscriptionsAsync` and
`ReactivateSubscriptionsAsync` fail with `BadSecureChannelClosed`
(retryable) instead of deadlocking. This covers the operation holding
the reconnect lock and any queued behind it.
- **Closing or disposing from a handler no longer waits on itself (L1-3,
G5, G6).**
- `Session` and `ManagedSession` Close/Dispose from a Notification
handler no longer wait 30 s for their own background drain.
- Disposing a `ManagedSession` from a keep-alive `PublishStatusChanged`
handler no longer deadlocks.
- Tasks spawned by a handler no longer count as "the handler" (the owner
marker is scoped to the work item).
- **Switching to an ECC user-token policy takes two ActivateSession
calls (L1-4, G4).** This follows Part 6 §6.8.2 (fetch the ephemeral key,
then activate).
- If the server returns no key, the switch fails with
`BadSecurityPolicyRejected`.
- If the second activation is rejected, the old identity stays on the
override policy with the key just fetched. Restoring the old key was
tried: against the reference server the next reconnect then failed to
decrypt the identity token.
- **`ManagedSession.ConfiguredEndpoint` follows failover (L2-6).** It
now reports the inner session's endpoint, so it changes after a failover
or network-path rotation.
- **An explicit identity update replaces the identity provider (L2-7).**
`ManagedSession.UpdateSessionAsync(identity)` stops the provider's
refresh loop and drops the provider for good. If the update fails, the
loop is restarted.
- **Reconnect waits for in-flight calls on a healthy channel (L2-3,
G8a).** Reconnect/failover on a healthy channel (certificate or
trust-list change, manual reconnect) waits for in-flight service calls.
The wait is bounded (max(`KeepAliveInterval`, 1 s)) only once the
channel is unhealthy (keep-alive stopped, channel not Ready), so a dead
channel no longer holds recovery for `OperationTimeout`.
- **Smaller fixes:**
- Restore reads the server certificate as a chain; this failed on macOS
(L1-5).
  - Session recovery can no longer run twice at once (L1-6).
  - Identity expiries more than 49.7 days out no longer throw (L2-2).
- Keep-alive reconnect suppression is recalculated on a lease swap
(L2-4).

### Reconnect state machine and reverse connect
- **⚠ `MaxTotalReconnectTime = 0` now means "no time limit" (L3-7).
Reviewer decision needed.** Before, 0 disabled reconnect entirely.
Negative values other than `Timeout.InfiniteTimeSpan` now throw
`ArgumentOutOfRangeException` when the state machine is constructed;
before, they crashed the worker. The XML docs of
`ReconnectPolicy`/`ReconnectPolicyOptions` are updated. If 0 should keep
meaning "no reconnect", this is a one-line revert.
- **A close racing the end of the reconnect budget is no longer lost
(L3-4).** It goes through the normal close instead of switching to
failover.
- **The "reconnect worker" marker covers one step only (L2-1).** It
lasts for a single connect/reconnect/failover/close step. Keep-alive,
publish and identity loops started in a step no longer count as the
worker afterwards.
- **Reverse-connect strategies `Undefined` and `Any` alone are removed
after the first callback (L8-1).** This matches the documentation;
before, they behaved like `Always`.
- **`AddEndpoint` for a URL that is already added disposes the replaced
listener (L8-2).**

### Service calls
- **Batched calls clean up after a failed later batch (L3-2).** When a
later batch of
`Browse`/`BrowseNext`/`HistoryRead`/`CreateMonitoredItems` fails, the
continuation points and monitored items created by earlier batches are
released or deleted before the exception is rethrown. This adds one
request. `AddNodes`/`RegisterNodes` are not undone.
- **Batched diagnostic infos point at the right strings (L3-3).** The
string-table indexes are rebased when diagnostics are padded.
- **`ReadBytesAsync` rejects oversized responses (L3-5).** It throws
`BadUnknownResponse` when the server returns more bytes than requested,
and `BadEncodingLimitsExceeded` beyond `int` range.
- **`ReadByteStringInChunksAsync` handles an unknown server limit
(L3-6).** When the server's `MaxByteStringLength` is 0 (absent) or
larger than `int`, it falls back to the client's
`MessageContext.MaxByteStringLength`. It throws only when both are
unknown.

### Browse and NodeCache
- **Empty browse pages with a continuation point are followed (L9-1,
L9-4, L9-5, G16).** Part 4 §5.9.2.1 allows a server to return an empty
page with a continuation point, so browsing keeps following it. After 10
consecutive empty rounds it gives up with `BadNoData` and releases the
continuation point.
- `BrowseAsync(NodeId)` now throws instead of silently returning a
truncated list.
  - `BrowseStreamAsync` yields a `BadNoData` result for that node.
- The same cap, through one shared helper, now also protects
`RoleManagementClient.ListRolesAsync` and the FileSystem
enumerate/empty-check loops.
- **Continuation points are released on failure (L9-3).**
`BrowseAsync(NodeId)` releases them on any exception, and
`BrowseStreamAsync` releases all points of the current page when the
consumer stops early.
- **NodeCache lookups propagate cancellation (L9-6, L9-8).**
`FindAsync`, `IsKnownAsync`, `FetchNodeAsync`,
`FindReferenceTypeNameAsync`, the reference-table fill and the
type-hierarchy search now let `OperationCanceledException` through.
Before, a cancelled lookup looked like "node does not exist".
- **A NodeClass mismatch fails only that node (L9-7).** It returns
`BadNodeClassInvalid` for that node instead of throwing
`KeyNotFoundException` for the whole batch.
- **Smaller fixes:**
- `MaxNodesPerBrowse` ≥ 2^31 no longer breaks the managed browse (L9-2).
  - Null discovery URLs are skipped (L9-10).
  - `ModelChangeTracker` no longer cancels a joined start (L9-11).
- **Log changes:** `BrowserContinuationPointExistsButBrowse` (Browser +
0) was removed, and `BrowserServerReturnedEmptyReferencesBut` now
includes the round count.

### Subscriptions (default engine)
- **Recovery after a transfer delivers each message exactly once (L5-1,
G22).** Messages already dispatched are acknowledged instead of
republished and delivered again. Messages whose earlier republish failed
are now republished and delivered, with the Republish flag and after
newer messages, instead of being acknowledged and lost.
- **A keep-alive that proves a gap triggers republish at once (L5-5,
G23).** The dedup mark moves only past messages that were actually
recovered, so a failed republish is retried by the next data message.
- **Item changes that fail are retried with backoff (L5-3).** They are
retried on the next apply pass after the state manager's 250 ms–5 s
backoff, instead of six times back to back. The six attempts now span
about 13 s.
- **Extreme revised intervals no longer throw (L5-4, L6-1).** Very large
or NaN intervals and counts revised by the server are capped (NaN falls
back to the requested value). Before, the overflow could stop every
publish worker.
- **`SetTriggeringAsync` from a callback no longer hangs (L5-7, G21).**
While the subscription is deleted, recreated or disposed, it returns
`BadOperationAbandoned`. A cancelled reset no longer makes later calls
fail.
- **Publish responses for unknown subscription ids (L6-3, L6-4, G20).**
The client no longer re-sends acknowledgements the server already
processed. An unknown id seen while a creation is pending is left alone
instead of being deleted.
- **Smaller fixes:**
- A null and an empty `IndexRange` are treated as equal, and restored
snapshots use `null` (L5-6).
  - A failed partition attach during restore is rolled back (L6-2).
- `StreamingSubscription` throws `ObjectDisposedException` after dispose
(L6-6).

### Subscriptions (classic `Subscription`)
- **Republish no longer abandons other missing messages (L7-1).** A
Republish response no longer overwrites `AvailableSequenceNumbers`, so
other missing messages are not given up while the server still holds
them.
- **Late copies are no longer delivered twice (L7-2).** Copies of
already-processed sequence numbers are ignored, and creating a new
server subscription resets the sequence state.
- **Disposing from a keep-alive handler no longer deadlocks (L7-3).**
`Dispose()` from a keep-alive `PublishStatusChanged` handler can return
while that callback is still finishing.
- **Transfer of a restored subscription (L7-4, G27).**
  - It succeeds even when some saved items were never created.
- The server subscription is deleted only when this session's own
TransferSubscriptions just moved it and adoption failed definitively
(item count mismatch or a non-transient status). Transient errors and
`ReactivateSubscriptionsAsync` never delete it.
  - The public `Subscription.TransferAsync` never deletes.
- **Smaller fixes:**
  - Cloned items keep their triggering links (L7-5).
  - `CreateAsync` clears stale server ids (L7-6).
- A `Good_SubscriptionTransferred` for the client's own transfer is
ignored (L7-7).
- An item removed while its create is in flight is deleted afterwards
(L7-8).
- The publish timeout for very long keep-alives no longer overflows
(L7-9).
  - A negative `MaxMessageCount` is handled (L7-10).
  - `LastMessage` tolerates a wrong notification type (L7-11).
  - Pending acks exactly 2^31 away no longer throw (L4-1).
- **Log changes:** new log events `Subscription + 68..70`.

### Redundancy
- **Warm/Hot demotion (L4-2, G1, G3).**
- Without token reuse, the demoted leader closes its session and
reconnects as a fresh standby.
- With token reuse, it abandons the shared server session locally,
without CloseSession, so the new leader can reactivate it and keep its
subscriptions. This uses the new internal methods
`ManagedSession.AbandonServerSessionAsync` and
`Session.AbandonServerSession`.
- `RoleChanged(false)` is now raised before the standby connect and no
longer depends on it, so `CurrentSession` can briefly be `null`.
- Exceptions from `RoleChanged` handlers are logged and do not stop the
teardown.
- **Promotion (L4-3, G2).** A promoted standby that adopts the leader's
session first closes its own server session. A retried promotion keeps
the session it already adopted.
- **Client failover takeover (L4-4, partial).** Subscriptions the backup
prepared in advance (not yet created, `TransferId` set to the active
client's id) are moved through the session's own transfer and keep
publishing. `docs/HighAvailability.md` is updated.
- **`ShouldFailover` through the interface (L4-6).** Calling it through
`IServerRedundancyHandler` no longer fails over away from a server that
is still Healthy.

### Fluent / dependency injection
- **Shared DI session (L10-5).** The shared session from `AddClient` is
reconnected after a caller disposes it, and is disposed with the
container. After that the connect delegate throws
`ObjectDisposedException`.
- **New public API:
`ManagedSessionBuilder.WithServerRedundancyOptions(ServerRedundancyOptions)`
(L10-6).** This is additive. The `Session.ServerRedundancy` options are
now actually applied.
- **`AddDiscoveryAndConnect` rewrites the advertised address (L10-7).**
It replaces the host and port the server advertises with those of the
discovery URL (Part 4 §5.6.2.1, NAT/containers), and prefers endpoints
with the same scheme.

### State machines and role management
- **`GetSubStateMachineAsync` returns the instance's own sub-state
machine (L10-1).** Before, it returned the type's placeholder.
- **`WaitForStateAsync` reports failures with OPC UA status codes
(L10-2).** It throws `BadNotFound` when `CurrentState/Id` cannot be
resolved, and `BadInvalidState` when observation ends without reaching
the state. Before, both surfaced as `OperationCanceledException`.
- **`RoleManagementClient.ListRolesAsync` pages correctly (L10-3).** It
follows continuation points, releases them on any failure, and throws on
a Bad browse status instead of returning an empty list.
- **Role properties are read into the right fields (L10-4).** Properties
whose namespace cannot be resolved are skipped instead of shifting the
other values.

### File system, temporary files and alias names
- **Path escaping (L11-8, G12).**
- Path segments escape `:` and `/` with `&`, and `&` itself as `&&`
before those characters or at the end of a name.
- `Format` and `Parse` round-trip all names. Typed input ending in `&/`
now reads the slash as part of the name.
- A segment without a namespace prefix also matches a namespace-0 child
when nothing matches in the parent's namespace.
- **Stale path cache (L11-1).** When a cached lookup fails,
`GetInfoAsync` retries once from the server. `ExistsAsync` and
`GetFileAsync` report "not found" instead of throwing.
- **`GenerateFileForReadAsync` waits for the file (L11-2).** It waits
until the file-generation state machine reaches ReadTransfer (`Idle`
counts as not started yet), and throws `IOException` if it reports
Error. The wait is bounded only by the caller's token.
- **Non-recursive delete refuses hidden children (L11-3).** It refuses
when the directory holds children that the subtype options hide from
enumeration. Before, the server deleted them all.
- **Alias names (L11-5, L11-6, L11-7).**
- `BadMethodInvalid` maps to `NotSupportedException` for all alias
calls, so `UseVerbose` falls back to `FindAlias`.
  - A duplicate alias name resolves to all its targets.
- If the refresh strategy fails to start, the resolver logs a warning
and keeps working without automatic invalidation. New log event block
`AliasNameResolver = 560`.
- **Null profile entries (L11-4).** Null `ServerProfileArray` entries no
longer crash `GetConformanceInfoAsync`.

### WebApi transport
- **Failures come back as `ServiceResultException` (L12-1).**
  - A timeout becomes `BadRequestTimeout`.
- HTTP 429/503 becomes `BadServerTooBusy` with the Retry-After hint, on
every target framework. `WebApiClient` checks the status itself (reusing
`HttpsTransportChannel.CreateServerTooBusyException`), so callers of
`WebApiClient` also get a `ServiceResultException` instead of an
`HttpRequestException` for these two statuses (G15).
  - An IO error becomes `BadNotConnected`.
- **Timeouts (L12-1, G14).**
- `OperationTimeout` applies to each request. When `RequestTimeout` is
unset, HttpClient's 100 s default no longer cuts off long Publish
requests.
- The GetEndpoints call at open is bounded by `OperationTimeout`, and
cancelling the open now throws.
- **Core change:** `HttpsTransportChannel.MapRequestFailure` went from
private to internal so the WebApi channel can reuse it.

### ComplexTypes
- **Out-of-range union selector (L12-3).** A union whose SwitchField is
larger than its field count now fails with `BadDecodingError` when
decoded and `BadEncodingError` when encoded (Part 6 §5.2.8).
- **Verbose JSON unions (L12-4).** They now decode their selected field
instead of `null`.

## Not fixed / partial
- **L2-5 (skipped):** an identity provider combined with a
caller-supplied reverse connection still can't connect to an
undiscovered endpoint. Fixing it means choosing the identity after
CreateSession, a design change.
- **L4-5 (skipped):** the one-time KeyCredential token needs a fresh
token on each reactivation. Today reactivation fails and the client
falls back to a new session.
- **L4-4 (partial):** only subscriptions the backup prepared in advance
survive a takeover.
- **L3-2 (partial):** `AddNodes`/`RegisterNodes` batches are not rolled
back.
- **L2-3 (partial):** service calls still hold the session lock for
their whole round trip.
- **Known from the review, not fixed here:**
  - G7: the batch cleanup request can't be cancelled.
- G11: the DI session's close at host shutdown never reaches the server,
and a closed session keeps being handed out.
- G13: a file renamed by another client is still found under its old
path.
  - G25: a clone-based transfer doesn't mark the original subscriptions.
  - G27b: the transfer item check matches by count, not by ID.
- **Outside this PR:** the reference server ignores `ECDHPolicyUri` on
ActivateSession (`SessionSecurityPolicyHelper`), which breaks Part 6
§6.8.2. As a result, the L1-4 two-step ECC switch ends in
`BadSecurityPolicyRejected` against our own server.

## Testing
All suites on net10.0, on the branch before the master merge. Every
suite had 0 failures:

| Suite | Passed | Skipped |
|---|---|---|
| Opc.Ua.Client.Tests | 2698 | 0 |
| Opc.Ua.Redundancy.Client.Tests | 134 | 0 |
| Opc.Ua.Subscriptions.Classic.Tests | 57 | 0 |
| Opc.Ua.Bindings.Https.WebApi.Tests | 385 | 0 |
| Opc.Ua.Sessions.Tests | 782 | 379 |
| Opc.Ua.Subscriptions.Tests | 640 | 6 |
| Opc.Ua.Client.ComplexTypes.Tests | 2858 | 0 |

After merging current master (net10.0), every suite had 0 failures
except one, a flaky new test fixed in 9f52952b2:

| Suite | Passed | Skipped | Failed |
|---|---|---|---|
| Opc.Ua.Client.Tests | 2719 | 0 | 1 (the flaky test) |
| Opc.Ua.Redundancy.Client.Tests | 134 | 0 | 0 |
| Opc.Ua.Client.ComplexTypes.Tests | 2858 | 0 | 0 |
| Opc.Ua.Subscriptions.Classic.Tests | 57 | 0 | 0 |
| Opc.Ua.Bindings.Https.WebApi.Tests | 397 | 0 | 0 |
| Opc.Ua.Sessions.Tests | 790 | 379 | 0 |
| Opc.Ua.Subscriptions.Tests | 640 | 6 | 0 |

The touched test projects also build on net48 with 0 warnings.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
 2026-09-29 09:34:03 +00:00
romanett ea9a8a07e1 Fix two test races: reverse connect reload commit and TCP chunk release (#4582) 
## Reverse connect reload commit race
(`ReverseConnectManagerLifecycleTests`)

Fixes the flaky
`CanceledStopRollbackRequeuesInFlightReloadAndAppliesChange` failure
seen on #4574 (run 36466677911, `test-linux-netstandard2.1 (5/30)`):

```
Expected: "Started"
But was:  "Starting"
```

### Root cause (test-only race)

The failing assert is the final `Started` check (line 3294), not the one
after the cancelled stop. The cancelled stop does restore `Started`
synchronously before it faults: the in-flight reload is still parked in
`GatedProvider` at that point, so no activation can publish `Starting`.

After `releaseReload`, the re-queued reload runs in the background.
`ActivateAsync` opens the candidate listener (`OpenHostsAsync`) and only
then creates the watcher and commits `Started` under the lock.
`WaitForAsync` returned as soon as the new listener was open, so the
next assert could observe the transient `Starting` state. The manager's
contract holds; the test only waited for part of the transition.

`WatcherTriggeredReloadReappliesReverseConnectOptionOverlay` has the
same shape: it waits for the reloaded listeners, then asserts `Started`.

### Fix

Both `WaitForAsync` predicates now also require
`manager.CurrentStateForTest == "Started"`. The new listener is open
*and* the state is `Started` only after the reload commits: a rollback
would close the candidate listener.

### Verification

- **Causality:** a temporary `Task.Delay(200)` right after
`OpenHostsAsync` in `ActivateAsync` made the original test fail
deterministically at line 3294 with exactly `Expected "Started" But was
"Starting"`. With the fix and the delay still in place, all 120
`ReverseConnectManagerLifecycleTests` passed. The delay was then
removed; the PR changes only the test file.
- **Stability:** `ReverseConnectManagerLifecycleTests` ran 10x on
net10.0 and 10x with `CustomTestTarget=netstandard2.1` (net8.0 host):
20/20 runs, 120/120 tests each.

## TCP chunk release race (`TcpMessageFailureRegressionTests`)


`ResponseSequenceFailureCompletesPendingRequestWithoutWaitingForTimeoutAsync`
asserts that `pool.Outstanding` is zero right after the request
completes. The request chunks, however, are released by
`HandleWriteComplete`, which runs on a pool thread after the transport
send has returned. When the response completes the request first, that
release can still be pending, and the assert sees outstanding buffers.

The test channel now overrides `HandleWriteComplete` to signal a
`WriteCompleted` task. The test waits for it (5 s bound) before it
checks the pool.

**Stability:** `TcpMessageFailureRegressionTests` ran 10x on net10.0 and
10x with `CustomTestTarget=netstandard2.1` (net8.0 host): 20/20 runs,
51/51 tests each.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-09-29 07:49:51 +02:00
Marc Schier 46fa7d364a Harden recovery, concurrency, and resource lifetimes across the OPC UA stack (#4516) 
# Description

Bound managed-session recovery, preserve Publish workers and recovery
ownership, and address the client, server, transport, and certificate
audit findings and subsequent concurrency review. Also reduce the memory
retained by exhaustive session-test diagnostics.

## Recovery and Publish ownership

- Add `ManagedSessionOptions.ChannelReconnectTimeout` through direct
construction, the fluent builder, and DI. The automatic duration is
`max(3 * KeepAliveInterval, revised SessionTimeout, OperationTimeout)`,
sampled at cycle start. Positive overrides are supported;
`Timeout.InfiniteTimeSpan` explicitly opts out. Raw sessions retain
their unlimited default.
- Add `CreateReconnectBudget(TimeProvider)` as a required
`IReconnectParticipant` member, as agreed in review. Custom participants
implement it and may return null for no participant-specific bound.
- Enforce the earliest participant/caller deadline through transport
reconnect, activation/recreation, and subscription restoration. Late
callers can shorten an active cycle but cannot extend it.
- Cancel and unwind recovery, fence late responses, and release channel
recovery ownership before the existing `Faulted` event hands control to
the outer reconnect/failover policy. Preserve session recreate admission
through final Publish cleanup and keepalive suppression until shared
recovery is complete.
- Treat participant budget creation/evaluation failures, including
exceptions at the exact deadline, as terminal shared-recovery failures.
Notify participants before exposing `Faulted`, preserve the original
diagnostic, and never silently replace a finite budget with unlimited
recovery.
- Keep caller-supplied replacement channels, managed bindings, wrapper
event subscriptions, and recovery-owner identities consistent across
success, failure, and cancellation. Reject replacement during active
recovery and fence retired callbacks so they cannot clear newer
ownership or suppress later keepalive recovery.
- Retain eligible Publish workers through temporary
subscription-identifier clearing without retaining workers for
never-created or removed subscriptions. Preserve cancellation,
acknowledgement rollback, and drain-before-replacement ordering.
- Coordinate automatic subscription updates with session recovery:
cancel admitted updates, keep them paused across a failed-restoration
handoff, and retry only after explicit restoration completes. This
prevents competing CreateSubscription requests and updates against a
retiring session, while preserving manual recreation, transfer fallback,
and pending option changes.
- Exercise the real ManagedSession, V2 subscription engine, and channel
ready gate with scripted transports and an injected clock. Cover
transfer fallback, deadline handoff, late completions, and resumed
notification delivery.

## Audit and review corrections

- Correct certificate/trust-list transactions, change-pump lifetimes,
validation, peer revalidation, and reset/resource ownership.
- Harden node admission, attribute/range/dimension validation,
monitored-item sampling and lifecycle transitions, event fanout, and
server shutdown. Run lifecycle readiness and compensation outside shared
monitored-item admission, with stale-completion ownership checks.
- Close synchronous node-manager admission before teardown and retain
resources until admitted service, lifecycle, and asynchronous method
work returns. Dispose the monitored-item manager outside both locks,
reject late calls before they can reach it, and forward asynchronous
disposal through the adapter so the master awaits the same completion.
Preserve reentrant shutdown initiation, cancellation/failure unwinding,
and shared cleanup-failure reporting.
- Bound WSS pending-request retention using connection-local wire
handles while preserving caller-visible request handles and translating
Cancel targets. Safely ignore retired replies and drain admitted
send/close users before resource disposal.
- Clarify that the shared `ClientBase` caller-handle sequence is
distinct from the non-wrapping WSS wire allocator. Existing
exhaustion/reopen, late-reply, and Cancel-translation regressions verify
the connection-local contract; no allocator behavior was changed by this
clarification.
- Preserve virtual-node validation and resolver service errors instead
of converting them to cached misses. A mismatched NodeId reports
`BadNodeIdInvalid`; callback type mismatches and resolver service errors
keep their exact status. Per-item boundaries preserve unaffected reads,
writes, history operations, method calls, browse paths, and
monitored-item creation/restoration, including permission-validation
paths. Caller cancellation still propagates; stored-item validation
failures are logged without aborting later restores.
- Correct client file/role/state-machine/streaming APIs, node-cache
behavior, redundancy/failover ownership, and session
configuration/identity handling.
- Cancel each owned peer-discovery scope before disposing it, including
when the bounded wait wins a cancellation-delivery race. Keep late
results out of the endpoint cache and preserve single-flight lookup
ownership.
- Keep materialized alias Browse results synchronized with the live
store by default, preserving explicit startup-only opt-out. Apply
HistoryUpdate service results per value while retaining explicit
all-or-nothing provider APIs.
- Update the directly related feature documentation and add focused
positive, negative, concurrency, and ownership regressions.

## Latest review-feedback corrections

- Fence stale failed lease observations with a successful-confirmation
generation distinct from the revocation epoch. Snapshot it before each
store read, including failed-CAS revalidation, so an old failure cannot
revoke or reschedule a newer lease while fresh ownership loss still
revokes leadership. Preserve independent overlapping acquisitions,
expiry, disposal, and late-success protections.
- Isolate attributable WSS response-decoding errors to their request,
including retired handles. Keep admitted socket sends connection-owned
so caller timeout/cancellation cannot abort sibling requests; shutdown
still cancels and drains those sends.
- Restart keepalive and classic Publish replenishment after successful
session reuse. Retire V2 worker demand after intentional
deletion/disable without confusing that state with temporary recovery
resets.
- Browse virtual directories lazily across continuation pages without
the 1,024-entry snapshot cap. Preserve active-page cancellation, bounded
metadata retention, pushback, and exactly-once asynchronous cursor
cleanup.
- Apply certificate/CRL staging in last-operation order, preflight
required issuer storage before writes, select actual policy-compatible
RSA keys for None endpoints, frame certificate chains independently of
signature OIDs, and preserve admitted validation results during
rejected-writer shutdown.
- Preserve base asynchronous disposal when a legacy override omits its
base call; avoid double derived disposal in the adapter. Let genuinely
admitted nested work finish while rejecting stale captured contexts and
returning null from closed lookup APIs.
- Re-park recovered monitored items if AddNodes rolls back. Give event
compensation a fresh finite deadline, and preserve healthy
Browse/Translate siblings at every reference/path boundary while
retaining starting-node errors and caller cancellation.
- Give final reconnect notifications a live, bounded shutdown-linked
lifetime. Synchronize child state before parent lifecycle handlers
outside the parent transition lock.
- Document the accepted existing policies: direct reverse connections
are single-use without a ReverseConnectManager, and multi-node stream
capacity is shared rather than reserved per node.

The expanded readiness controls also exposed a real generation-handoff
race: an old source could finish draining after activation was deferred
but before readiness tasks were collected, causing the wanted
replacement to be omitted from the wait set. A deterministic test gates
that exact interval; the fix keeps the replacement in the readiness
wait. The original race passed 200 repetitions after the fix. A separate
control now awaits both producer readiness and iterator entry under the
same original deadline, rather than treating those independent signals
as equivalent. Temporary logging and repetition are removed.

## CI integration contracts

The History and Server CI failures exposed integration tests that still
asserted the behavior intentionally changed by the audit fixes. A
Windows WoT negative test raced its caller deadline against upstream
retries. A subsequent Ubuntu Client failure exposed a real
automatic-update versus explicit-restoration race:

- History insertion collisions now verify exact per-value statuses and
all persisted timestamps/values, including preservation of the existing
conflicting entry. Explicit atomic-provider rollback controls remain.
- Alias materialization tests wait for actual publication and verify the
added node plus forward/inverse `AliasFor` references, instead of racing
a startup-only snapshot expectation. Explicit refresh opt-out controls
remain.
- The unavailable-upstream WoT test uses an explicit finite retry policy
through existing DI configuration, preserving its original four-minute
guard and requiring a mapped-value service error before caller
cancellation. Production retry defaults remain unchanged.
- The Client fix retains the exact one-create and late-response
assertions. Three deterministic real-subscription/real-manager cases
cover paused creation, disposal while paused, and cancellation of an
admitted create followed by explicit restoration and a pending option
update. Both composed restoration-deadline cases also prove automatic
updates stay blocked at the outer handoff and resume after restoration.
- The merged-head net48 correction prevents lost peer-resolver
cancellation and distinguishes the harness's delayed initial `Connected`
event from actual outer recovery completion. A deterministic timer-order
test covers both cooperative and cancellation-ignoring resolvers,
cancellation before return, rejection of late cached results, and a
fresh subsequent lookup.
- The remaining CodeQL alert was a false content flow through
`ArrayOf.Count` into an integer-only trust-list summary. Projecting the
same count through `Span.Length` keeps cardinality distinct from
serialized contents without allocating, dropping diagnostics,
suppressing the rule, or dismissing the alert. Five log-contract cases
cover null/empty/nonempty results and mixed chain-capable/fallback
listeners, asserting the exact count, message, and structured
properties.
- Classic transfer acknowledges messages selected for asynchronous
Republish only after their responses arrive. The existing locked
replay-selection decision returns the immediate-acknowledgement subset
to Session, preserving the public transfer API, advertised availability,
and immediate acknowledgement of older gapped messages not selected for
replay. A callback changing `RepublishAfterTransfer` cannot change the
already-selected acknowledgement ownership.
- Transfer integration tests explicitly control acknowledged versus
retained origin messages. Exact assertions distinguish retained replay
from requested fresh initial values by sequence number, client handles,
values, status, and source timestamp. Atomic counters, observed
delivery/keepalive progress, and failure-safe session/file cleanup
replace timing assumptions and racy resets.
- Server lifecycle tests verify the documented independent five-second
compensation deadline rather than requiring a non-cancellable token.
They cancel the actual request, prove compensation and unrelated
monitored-item creation still proceed one tick before the deadline,
preserve the original failure and ownership assertions, and verify the
completed deadline is retired.
- Synchronous and asynchronous directory-browser tests require the exact
provider exception instead of an empty-directory success, then verify
the failed cursor is not enumerated again. These correct expectations
left behind by the intentional, documented provider-error propagation
change; production behavior is unchanged.

No tests were disabled, assertions loosened, coverage thresholds
lowered, or CI checks made optional. Existing platform exclusions and
Explicit debug variants remain.

## Related Issues

- Fixes #4514
- Fixes #4515
- Builds on merged #4503 and retains the #4508 publishing-drain
behavior.

## Validation

**Lease-election review correction
(`a13afa641182c70983e16bbcfebf507e666a45a5`):**

The correction is based on the existing upstream merge
`43079e2b851fe338a4082066f8d14abfdafb6e32`. Three deterministic
stale-verdict
cases fail on the original source and pass with separate confirmation
fencing.
Six new cases cover the initial foreign-owner verdict, failed-CAS
revalidation
in both invocation orders, and fresh ownership loss through both read
paths,
including revalidation begun after another successful confirmation.

| Target and configuration | Passed | Failed | Skipped |
| --- | ---: | ---: | ---: |
| net10.0 Debug | 120 | 0 | 0 |
| net48 Debug | 120 | 0 | 0 |

The selection is the full `Opc.Ua.Core.Tests.Redundancy` namespace,
including
all 38 `SharedStoreLeaseElectionTests` cases and the existing expiry,
clock
rollback, disposal, and overlapping-success regressions. New witnesses
are
`StaleForeignLeaseReadDoesNotRevokeNewerAcquisitionAsync`,
`StaleFailedRevalidationDoesNotRevokeNewerAcquisitionAsync`, and
`FreshForeignLeaseObservationRevokesLeadershipAsync`. They assert
resulting
leadership, exact transitions, independent progress, and the unchanged
expiry
boundary. Retaining the initial generation for the fallback read
independently
makes the fresh-loss-after-confirmation control fail; that mutation was
removed
before the final rebuilt runs.

The successful scoped command is
`dotnet test tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj -c Debug
--no-restore --filter
"FullyQualifiedName~Opc.Ua.Core.Tests.Redundancy"`,
with `CustomTestTarget` and `-f` matching each row and that framework's
assets
restored. Both changed C# files pass formatting and analyzers through
informational severity on both frameworks. These are scoped results, not
a
full-solution test or measured coverage-delta claim. CI monitoring is
paused
for this feedback-only operation; no current-head CI success is claimed.

**Latest CI contract correction
(`1cd1f2047bb5b50c9bf300b1ff710b0b2e935a29`):**

All four failing Server shard 23 jobs in
[run
36120946740](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36120946740)
reported the same six cases. Two local replays reproduced all six
failures.
The source changes and documentation from `cd43b831a` establish both
intentional
contracts: independent bounded event compensation and surfaced provider
errors.
Only the two test files are corrected; production code and CI policy are
unchanged.

| Target and configuration | Passed | Failed | Skipped |
| --- | ---: | ---: | ---: |
| net48 Debug | 113 | 0 | 0 |
| net10.0 Release, CI Coverlet settings | 113 | 0 | 0 |

The selection contains `MonitoredItemManagerLifecycleTests`,
`DirectoryObjectStateTests`, `DirectoryBrowserBoundedEnumerationTests`,
and

`EventCompensationReadinessHasFiniteDeadlineWithInfiniteSourceTimeoutAsync`.
Both changed files pass formatting and analyzers through informational
severity
on both frameworks. Independent mutations restoring a non-cancellable
compensation token and swallowing provider errors fail all four
lifecycle
cases and both browser cases, respectively. Both mutations were removed
before
the final passing runs and commit.

The scoped command is `dotnet test
tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj`
with `CustomTestTarget` and `-f` set to the selected framework,
`--no-restore`, and
`--filter
"FullyQualifiedName~MonitoredItemManagerLifecycleTests|FullyQualifiedName~DirectoryObjectStateTests|FullyQualifiedName~DirectoryBrowserBoundedEnumerationTests|FullyQualifiedName~EventCompensationReadinessHasFiniteDeadlineWithInfiniteSourceTimeoutAsync"`.
The modern run additionally uses `-c Release /p:CollectCoverage=true
--collect "XPlat Code Coverage" --settings
tests\coverlet.runsettings.xml`.
These are scoped results, not a full-solution test or measured
coverage-delta claim.
Hosted checks for that commit were pending at publication. The fail-fast
CI
watch was stopped when the requested operation switched to review
feedback;
no current-head CI success is claimed.

**Earlier master merge (`8b15ea6ef9fc2214ac8ec886f373bb3846dd6d6f`, base
`b4136d8e9bab0df680f41d9ef985f519160534b1`):**

The `TcpListenerChannel` conflict preserves both lifetime
responsibilities:
disposal clears the retained peer-certificate chain and unregisters the
quota-scoped message-assembly scheduler. Both fields and all upstream
deadline
handling remain intact. The only additional manual source adjustment
adds the
parentheses required by the existing analyzer around the session-count
fallback.

Master advanced during validation. The initial transport merge
`80303c434ce813cf75aa7fe54a6a32fe2aeafc30` was retained, and the newer
base was
merged normally in `8b15ea6ef9fc2214ac8ec886f373bb3846dd6d6f`, without
rewriting
history. The additional recovery, fluent-pipeline, and test-framework
overlaps
were inspected; the branch's cancellation, validation-error, and
acknowledgement
ownership fixes remain alongside the upstream changes.

Full `UA.slnx` builds passed on net10.0 and net48 with **0 warnings and
0 errors**.
The final scoped tests rebuilt against the committed tree
`2074f8efcb61d8d9c6106e7d6f76ee941940e358`:

| Target | Passed | Failed | Skipped |
| --- | ---: | ---: | ---: |
| net10.0 | 2,653 | 0 | 0 |
| net48 | 2,529 | 0 | 7 |

The net48 skips are existing platform exclusions: six RSA_DH_AesGcm
nonce cases
and one ClientWebSocket certificate-validation case requiring .NET 7 or
later.
The selection covers transport cleanup, reassembly
budgets/deadlines/admission,
peer-chain ownership, server session binding, fluent lifecycles and
source
generation, reconnect and keepalive, transfer acknowledgements, Publish
queues,
and JSON encoding. Modern-.NET WSS transport and real-server integration
cases
also pass. The resolved file passed solution-relative formatting and
analyzer
verification through informational severity on both frameworks.

These are scoped test results, not unfiltered full-suite or
measured-coverage
claims. Hosted CI was not monitored or repaired as part of this
merge-only
operation.

Selected local results below are overlapping validation runs, not
distinct-test totals.

**Earlier review-feedback batch
(`cd43b831a8f11de3f7d65c3681712a61e3c669a5`):**
The final net10.0 scoped selection passed **471 tests, with no failures
or skips**.
The final net48 solution-root selection passed **439 tests, with no
failures and
one unchanged platform skip**
(`ValidateServerCertificateFallsBackToTlsPolicyErrors`,
which requires newer ClientWebSocket certificate-validation support).
Both full nonincremental solution builds completed with **0 warnings and
0 errors**,
and all **40 changed C# files** passed informational-level
formatting/analyzer
verification. Both frameworks validated the identical final tracked
diff.
The focused matrix covers every one of the 20 current review threads,
including
real mid-frame WSS cancellation, stale admission contexts, directory
continuation
ownership, multi-element Browse/Translate failures, and the readiness
drain race.
The net10.0 solution-root test command has a separate pre-existing
runner conflict:
three unrelated AOT projects reject the VSTest target under
Microsoft.Testing.Platform
2.4.0 and SDK 10. Per-project net10.0 results are recorded separately;
no runner
migration, test exclusion, assertion relaxation, or CI-policy change is
included.
These are bounded selections, not unfiltered full-suite or measured
coverage claims.
At publication of `cd43b831a`, hosted CI was not green: the
`OPCFoundation.UA-.NETStandard` check reported `ACTION_REQUIRED`,
AppVeyor reported
`FAILURE`, and the two signed-build jobs were in progress. No CI jobs
were rerun,
cancelled, approved, or repaired as part of that feedback-only
operation.

**Classic transfer correction
(`f425376fca045794122e19aaf9cafebc4a717d91`):**
The preceding head's
[macOS net10.0 shard
25/30](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/35980326299/job/107570512458)
failed
`TransferSubscriptionTest(DisconnectedRepublish).TransferSubscriptionAsync(True,False)`
at the static-node count assertion: expected 10, received 20. The trace
contains a successful,
explicitly requested republish of retained sequence 1 and a fresh
initial-value batch at sequence 3,
not duplicate dispatch of one response.

Controlled lost origin acknowledgements reproduced that incorrect test
expectation on both the
preceding PR head and exact master base
`a2539cb53dd1666cef856f33812d7a8830d68909`.
The maintainer explicitly authorized correcting this existing issue in
the PR. Retained replay
and requested initial values are independent, as specified by
[OPC UA Part 4,
TransferSubscriptions](https://reference.opcfoundation.org/specs/OPC-10000-4/5.14.7).
The reason that the original hosted run retained its particular origin
message remains unproved.

The expanded full-suite run then exposed a **real production race**, not
just a count assertion:
Session queued acknowledgements for all transferred available messages
before the asynchronous
Republish responses arrived. Publish could delete those messages first,
causing
`BadMessageNotAvailable`. A gated real-Session/classic-engine regression
failed both replay-enabled
cases on the original implementation while both no-replay controls
passed. The ownership fix
passes all 24 combinations of empty/consecutive/unsorted-gapped queues,
replay preference,
sequential delivery, and a reentrant preference change.

All 40 normal real-server transfer cases pass on both frameworks.
Independent mutations
suppressing actual replay and duplicating observed sequence records each
fail the intended
assertion; both mutations were removed. The full classic suite passes on
both frameworks.
Hosted CI was pending when that correction was published; the earlier
failed hosted job was not
rerun for a lucky pass.

**Local net48 caveat:** the full Client run timed out in the
pre-existing fake-clock test
`ReconnectAsyncSwapsFaultedLeaseEntryAsync`, which does not exercise
subscription transfer.
Its unchanged isolated replay passed, but that does not resolve the
full-suite timeout.
No unrelated code or assertions were changed, and a clean full Client
net48 run is not claimed.

| Scope | net10.0 | net48 |
| --- | --- | --- |
| Full Client.Tests after transfer acknowledgement correction | 2,549
passed | 2,552 passed, 1 timeout failure noted above, 1 existing
platform skip |
| Full classic-subscription project after transfer correction | 57
passed, including all 40 normal transfer cases | 57 passed, including
all 40 normal transfer cases |
| Gated transferred-message acknowledgement ownership | 24 passed,
included above | 24 passed, included above |
| Saved-session consumer of the shared subscription helper | 40 passed |
16 passed, 24 existing unsupported-security-policy skips |
| Audit regression matrix | 2,339 passed | 1,934 passed, 34 existing
skips |
| Full Client.Tests after review follow-ups | 2,520 passed | Focused
selections only |
| Full Client.Tests after CI recovery correction | 2,523 passed, CI
coverage collector | 2,527 passed, 1 existing platform skip |
| Latest merged-head recovery/redundancy selection | 139 passed | 139
passed |
| Full Client.Tests after merged-head net48 correction | Focused
selection above | 2,529 passed, 1 existing platform skip |
| Lifecycle and actual-dimension regressions | 146 passed | 146 passed |
| Focused recovery follow-ups | 7 passed | 7 passed |
| WSS-related fixtures | 93 passed | Not applicable |
| CI History integration corrections | 28 passed | 28 passed |
| CI alias, dispatcher, and explicit-atomic controls | 63 passed | 63
passed |
| Full WoT sample project with CI coverage collector | 29 passed | Not
applicable |
| Trust-list effects and count-only summary contract | 22 passed | 22
passed |
| Virtual-node feedback, node-manager, dispatcher, and lifecycle
selections | 661 passed, 63 existing not-executed cases; all 48
virtual-node cases passed | 661 passed, 63 existing not-executed cases;
all 48 virtual-node cases passed |
| WSS wire-handle exhaustion, late replies, and Cancel translation | 9
passed | Not applicable |
| Synchronous node-manager disposal and related service/lifecycle suites
| 1,037 passed, 64 not-executed cases from existing fixture assumptions;
all 93 sampling/disposal cases passed | 1,037 passed, 64 not-executed
cases from existing fixture assumptions; all 93 sampling/disposal cases
passed |

- Full solution builds passed on net10.0 and net48 during the audit
work. The subsequent full non-incremental net10.0 build also completed
with zero warnings/errors.
- Modified source/test formatting and analyzer checks passed through
informational severity.
- Original composed recovery regressions passed repeated runs on both
frameworks. Independent mutations removing Publish cancellation,
bypassing the recreate drain call, or ignoring ready-gate cancellation
each failed the intended assertion; all mutations were restored before
passing runs.
- The latest recovery correction passed 100 executions of each original
restoration-deadline scenario with unchanged assertions. Restoring
premature update admission independently made both strengthened handoff
cases fail; the correct retention behavior was restored afterward.
- Startup-event and cancellation-delivery timing probes reproduced the
subsequent net48 failures, then passed 100 executions of each of the
three scenarios with the fixes. All timing probes and repetition were
removed. The permanent timeout-order regression also went RED on the
original cancellation cleanup without sleeps or scheduling
amplification.
- Final post-format recovery/subscription/engine checks passed all 151
selected cases on net10.0. The first local full net48 attempt aborted in
`clr.dll` with native access violation `0xc0000005`; an
unchanged-assembly full diagnostic rerun passed as recorded above. That
native host crash has not been attributed or claimed fixed.
- A minimal replay with the same CodeQL CLI and query-pack versions
reproduced the custom-count alert and confirmed that the standard
span-length projection is clean while direct sensitive-value and
raw-identifier logging controls remain flagged. The source was already
logging only a count; no identifier or certificate disclosure was
established at the reported location.
- The virtual-node feedback regressions first reproduced five
error-preservation/cache failures; expanded service-dispatch cases
exposed two additional whole-batch failures before the
operation-boundary fixes. The final selections pass on both frameworks.
Genuine misses remain cached, successful results remain reusable, failed
validation allows same-cache retry, and request cancellation still stops
the batch.
- Three deterministic synchronous-manager disposal regressions
reproduced early resource release and lifecycle calls reaching a manager
during disposal. The fix passes direct and adapted calls, a
sampling-worker join after the last operation exits, reentrant shutdown,
cancellation and failure paths, repeated disposal, and exact rejection
across 39 service/lifecycle entry points both during drain and after
cleanup. The original outside-node-lock disposal regression remains and
now requires the worker to observe closed admission.
- Prior-head CI on `c985f8bded77a60449d9cd381c8a004d8590ccf6` completed
with **168 passing checks, one expected release-package-publication
skip, and no failures or pending checks**. All test shards, signed
builds, container builds, the coverage check, and the CodeQL PR check
passed. Hosted CodeQL analysis `1829262912` completed successfully and
marks count-flow alert 385 fixed. This is evidence for that head, not a
claim that subsequent review follow-ups have completed hosted CI.

### Session-test diagnostic memory

Complete browse listings are streamed to unique NUnit attachments rather
than retained in inline NUnit/VSTest output. Detached regression-test
results are cleaned up without deleting runner-collected attachments.

- Two covered session-suite runs each completed with 782 passed and 379
existing skips. Peak working set fell from 4.86 GiB to 2.44-2.78 GiB.
- TRX output shrank from 303.1 MiB to approximately 40.7 MiB, retaining
all 46 complete browse attachments.
- Seven output regressions cover exact content, empty/single/large
listings, disabled/enabled output, bounded inline summaries, and unique
filenames.

The separate Azure build-18452 VM deprovisioning cause remains unproved
without provider logs. The memory measurements do not establish its
cause.

Unfiltered local `UA.slnx` tests, including the long stress/soak suites,
have not been run on both frameworks. A local coverage delta was not
collected; builds and focused tests are not substitutes for those
claims.

## 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.
- [ ] All checks on the latest review-follow-up head have completed
successfully; prior-head CI passed as recorded above.
- [x] I have addressed **all** PR feedback received.

Tests have been added and pass as recorded above; the hosted coverage
check passed on the prior head identified above. The combined
tests/coverage-increase checkbox remains open because no local coverage
delta was measured.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 68f26810-22b8-476f-abcd-0a8c82d6e208
 2026-09-25 14:59:34 +00:00
romanett 5d26f6f439 Fix JSON encoding fidelity issues found by fuzz crash replay and align with Part 6 (#4540) 
# Description

The external crash corpus replayed by `Opc.Ua.Encoders.Fuzz.Tests` still
produced several hundred tolerated *encoding fidelity findings* per run.
They came from the 10 BinaryJson conversion targets (binary decode →
JSON encode → JSON decode). None of them was a crash or an unexpected
exception. Triage reduced them to three JSON encoder defects plus one
oracle rule that was stricter than the specification. While checking the
fixes against OPC 10000-6, two adjacent deviations turned up and are
fixed as well.

### Encoder fixes (`Opc.Ua.Types`)

| Defect | Before | After | Spec |
|---|---|---|---|
| LocalizedText with no Text and an empty/null Locale | `{"Text":null}`
(Verbose) / `{}` (Compact). Reads back as null and re-encodes
differently | JSON `null` (Verbose) / omitted (Compact) | 5.4.2.15 Table
39, 5.1.2 Table 1, 5.4.6 Table 45 |
| LocalizedText with empty Text | `"Text":""` written | Text not
encoded; `LocalizedText("")` is the null LocalizedText | 5.4.2.15 Table
39 ("not encoded if it is null or empty") |
| Variant/DataValue holding a **null array**, Compact | `Value` omitted
→ decoded as a *scalar* default (array rank lost) | `"Value":[]` |
5.4.2.17 Table 41, 5.1.11 |
| Variant/DataValue `Value` omission rule | Omitted for any default
value (including null *arrays*) when `IgnoreDefaultValues`;
`IgnoreNullValues` ignored | Same rules as every other field: a NULL of
a nullable built-in type is omitted when `IgnoreNullValues` or
`IgnoreDefaultValues` is set (like `WriteNull`); a default of a
non-nullable type only when `IgnoreDefaultValues` is set (like
`WriteValueProperty`). An array `Value` is never omitted | 5.4.2.17
Table 41, 5.1.2 Table 1, 5.4.6 Table 45 |
| `ExpandedNodeId.Format(context)` with a ServerIndex/NamespaceUri and a
null identifier | Returned `null`, so JSON wrote `null` and the server
index was lost | `svr=N;i=0` / `nsu=...;i=0`, consistent with the
`IFormatProvider` overload | 5.1.12 Table 6, 5.4.2.11, 5.1.2 Table 1
(NULL is exactly `i=0`) |

### Fuzz oracle and harness (`fuzzing/`)
- The round-trip oracle now treats null and empty arrays, and null/empty
LocalizedText Text/Locale, as equivalent in every mode (5.1.11:
"semantically the same for all DataEncodings… applications shall treat
null and empty arrays as equal"). Previously this only held when
`IgnoreNullValues` was set.
- `FuzzCrashAssets` no longer prints a stack trace or an inline base64
reproducer for *tolerated* fidelity findings. Only the one-line summary
entry is reported, and it now names the actual cause instead of
`TargetInvocationException`. Before, tolerated findings used up the
60-per-target reproducer budget, so a genuine failure later in the
enumeration printed no reproducer. The externally supplied corpus inputs
were also written into the pipeline log on passing runs.
- Three hand-crafted curated reproducers (9–23 bytes, one per encoder
defect) were added under `Assets/Repo/`, which is always enforced
strictly. Each one fails on the previous master and passes with this
change.

### Reviewer notes: observable JSON output changes
- `LocalizedText("")` / `("en","")` now round-trip with `Text == null`
instead of `""` (equivalent per 5.1.11).
- Verbose and Compact scalar Variant output is unchanged.
`IgnoreNullValues` alone now also omits a NULL `Value`, as it does for
every other null field.
- A null *array* in a Variant is written as `"Value":[]` in every mode
instead of being dropped in Compact, and a null *matrix* is written as
`"Value":[]` instead of failing with `BadEncodingError`.
- Variants holding a Variant or DiagnosticInfo still reach the writer
and are rejected as before (`WriteBadVariantThrows`).

### Testing (net10.0, locally)
- Opc.Ua.Types.Tests 10,542 passed, including new JsonEncoder tests for
each rule above
- Opc.Ua.Core.Encoders.Tests 2,929 passed (one expectation updated from
`"Text":""`)
- Opc.Ua.PubSub.Tests 1,369 passed; Opc.Ua.Core.Tests JSON subset 83
passed; Opc.Ua.Aot.Tests 178 passed
- Opc.Ua.Encoders.Fuzz.Tests 8,112 passed; crash-asset tests in the
Certificates/Network/PubSub fuzz projects and Opc.Ua.Fuzzing.Tests
passed
- net48: `Opc.Ua.Encoders.Fuzz.Tests` builds with 0 warnings; net48
tests were not run locally

## Related Issues

- No tracking issue; found by triaging the fuzz crash-asset replay
output of the master pipeline.

## 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.
- [ ] 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.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-09-25 07:49:52 +02:00
Marc Schier 138086863b Fix audit findings and review regressions across the OPC UA stack (#4503) 
## Scope

Audit fixes and review follow-ups across the OPC UA Core, Client,
Server, transport, and redundancy components. This includes implemented
Tier 1, Tier 2, and Tier 3 work; it does not claim that every item in
the 244-finding audit backlog is complete.

The review batch fixes mandatory full-key record protection and owned
plaintext buffers; transactional pending-certificate claims;
shared-channel nonce and certificate ownership; discovery and
token-policy validation; reconnect/callback ordering without changing
unbounded buffering defaults; server permissions, filters,
monitored-item lifetime, and alarm topology; historian paging,
retention, and annotations; atomic user/password metadata; and canonical
file identities with bounded alias and role refresh workers.

## Review-batch verification at `ab686ccae`

The following results come from fresh test builds using `dotnet test`,
`--no-restore`, `-m:1`, and an explicit `CustomTestTarget`. They replace
the earlier description's library-build/`--no-build` totals, which did
not establish the state of the merged test sources.

| Scope | Target | Passed | Skipped |
|---|---|---:|---:|
| Core review regressions | net10.0 | 827 | 0 |
| Client and lifecycle regressions | net10.0 | 630 | 0 |
| Server review regressions | net10.0 | 1970 | 0 |
| Redundancy Server / Client / PubSub | net10.0 | 276 / 10 / 11 | 0 |
| Sessions / file-and-alias / streaming / HTTPS-WSS integration |
net10.0 | 8 / 21 / 30 / 20 | 0 |
| Core review regressions | net48 | 786 | 39 |
| Client recovery and policy regressions | net48 | 54 | 0 |
| Server ownership, aliases, queues, and user-state regressions | net48
| 137 | 0 |

Every listed run exited successfully. The 39 net48 skips are existing
platform/capability cases. New key-agreement tests assert
unavailable-policy behavior on unsupported targets rather than adding
skips or pretending those targets performed an unsupported handshake.
Additional exact node/permission/filter/alarm checks passed 13 Core and
46 Server cases.

The PR-scoped formatter verification covers 26 projects and 389
non-generated C# files. A Roslyn syntax audit covers 53 changed
interfaces and 263 members, with no missing
parameter/type-parameter/return contracts and a 120-column limit for
their declarations and documentation. No full-solution,
all-target-framework, or NativeAOT matrix is claimed from these local
runs; the new CI run must establish its own result.

## CI follow-up at `e1bc3ac3b`

The follow-up fixes the failures reported by Azure build 18400. TLS
certificates retain configured DNS names, including mixed-case URI
schemes, without relaxing hostname validation. Reverse-WSS fixtures use
the DNS identity in the client certificate. Reconnect completion is
published only after cycle cleanup, so a subsequent reconnect cannot
join an old completed cycle and retain old certificates.

Certificate-alarm roots now support event subscriptions. The pump sample
has one continuous notifier path, with no duplicate shortcut. Historian
and aggregate fixtures share the provider's retention clock. Alarm
collection lets the server expire idle Publish requests rather than
canceling only the client wait. The corrected server, WoT, and
managed-session fixtures exercise valid requests and initialized
lifecycle contracts. Downlevel crypto tests preserve real credential and
key-ownership assertions without enabling unsupported raw-ECDH policies
or adding skips.

Fresh local evidence after integration includes:

| Scope | Target | Passed | Skipped |
|---|---|---:|---:|
| Client lifecycle / channel ownership / builder fixtures | net10.0 and
net48, each | 96 / 103 / 123 | 0 |
| Combined server lifecycle, events, aggregates, crypto, and contracts |
net48 | 358 | 0 |
| Full lifecycle plus certificate-alarm and aggregate fixtures | net10.0
| 116 | 0 |
| Final server-contract and history-routing delta | net10.0 and net48,
each | 84 | 0 |
| Subscription / XRegistry / WoT event regressions | net10.0 | 7 / 2 / 3
| 0 |
| Full Sessions project | net10.0 | 780 | 379 |
| Full ComplexTypes project / downlevel resolver fixtures | net10.0 /
net48 | 2854 / 15 | 0 |
| Full WoT project / downlevel structured-write fixtures | net10.0 /
net48 | 1496 / 27 | 0 |
| TLS domain-name regressions | net10.0 and net48, each | 21 | 0 |

All listed runs exited 0. The Sessions skips come from existing endpoint
and policy capability guards; they are not counted as passes. No skip
was added. Worker validation also covered full DI (426 passes), full
History (540 passes, 21 existing skips), 38 exact downlevel
event/history failures, and 60 repeated certificate-rotation race cases.
The final crypto fixtures passed 73 cases on each target with the
unchanged leak detector; a temporary premature-disposal mutation made
all three borrower tests fail and was then removed.

All 29 C# files in this follow-up pass included-file formatting at info
severity across 12 projects. The complete PR interface audit now covers
54 interfaces and 271 members with zero missing contracts or line-width
findings. No full-solution or NativeAOT matrix is claimed. New-head CI
has started and remains authoritative for the remote result.

## Test-result gate follow-up at `4b6e1203a`

Azure build 18401 reported the net48 Client job as successful despite a
test-host crash after only 264 passing tests. Its old evaluator counted
per-test failures but ignored the failed run verdict. This was a partial
run, not a benign exit after the suite had completed.

The shared result evaluator now rejects aborted, errored, incomplete,
malformed, or inconsistent TRX reports. Each file must describe a
completed run, so a passing file cannot mask another file's failure. It
retains support for completed runs followed by a benign nonzero host
exit, existing skipped-result conventions, and no-TRX MTP jobs.

All 78 regression cases pass after integration using Windows PowerShell
5.1. Real completed Sessions and Client reports also pass. The
process-test fixture follows the existing NET10_0-only tools-test
pattern; its net48 build is not counted as a test run. Formatting passes
for the added test file.

The Client host crash itself has not reproduced locally. The isolated
fixture passed 59 tests, and two fresh full fast-CI selections each
completed with 2,392 passes and three existing skips, including a run
with crash collection enabled. The CI merge tree is identical to the
tested source tree. No speculative runtime change was made, and the
original partial CI run is not claimed as a full-suite pass. Azure build
18402 has now completed the full net48 Client run: 2,392 passed, three
existing console skips, and a successful test-host exit. The corrected
evaluator independently validated outcome=Completed. Across the matrix,
119 test-project jobs succeeded; Sessions net10.0 was canceled when the
remote provider deprovisioned its runner, so the overall Azure pipeline
was canceled. That cancellation is not claimed as a code failure or a
successful run. CodeQL and three Docker image jobs were still running at
the latest check. The migration discovery failure remains separate.

## Documentation follow-up at `57ed02bea`

The environment blocker has been removed by the maintainer. The three
bundled migration references now exactly match the canonical documents
after deterministic link rewriting; all 15 bundled documents pass
`validate-migration-plugin.ps1`. The canonical migration prose was not
rewritten. `FileSystemClient.md` now documents that extended Windows
roots preserve literal trailing-dot names, completing the filesystem
review follow-up.

The filesystem thread has been replied to and resolved after
publication. The review pass now has 108 of 110 threads resolved. The
two explanation-only threads remain open as requested; no unfinished
change request is being represented as resolved.

## Cross-platform follow-up at `0d9d936a8`

With migration discovery restored, the Linux and macOS test matrix
exposed two stale durable-subscription expectations. The expected value
83,442 encoded the old 32-bit overflow when converting 3,600 hours to
milliseconds. The existing overflow-safe server correctly returns
14,400,000 lifetime intervals at a 900 ms publishing interval. Only
those two expected constants changed; production behavior,
negotiated-hour checks, and all other assertions remain unchanged.

The exact local lifetime selection reproduced both failures before the
correction. The full durable suite now passes 36 cases with 11 existing
skips on each of net10.0 and net48. All four lifetime cases executed and
passed on both targets, and included-file info-severity formatting
passes.

## Session shutdown follow-up at `8daf7f091`

A late net48 Sessions failure exposed a shutdown race. Canceling an
in-flight keepalive read can produce a transport
`ServiceResultException`. The retiring worker treated its own
cancellation as proof that the peer was unavailable, so `CloseAsync` or
`DisposeAsync` could skip the server-side `CloseSession` request. The
worker now ignores that service error only when its own token is
canceled; genuine peer errors still update liveness.

The full integration fixture reproduced the original error-code
mismatch. Four deterministic mock-transport cases then produced three
failures and one passing genuine-error control before the fix. They
verify successful close, returned server error, asynchronous disposal,
and unchanged behavior for a real keepalive failure. Fresh related runs
pass 75 client and 101 integration cases on each of net48 and net10.0,
with no skips. Ten repetitions of the formerly flaky full net48 fixture
pass all 1,010 cases. All three changed files pass info-severity
formatting, and temporary diagnostic instrumentation was removed.

## Atomic listener snapshot follow-up at `82dc22b5d`

Ubuntu Core CI exposed an unsafe formatter rewrite in TCP listener
disposal: `ConcurrentDictionary.ToArray()` had become a collection
spread. A concurrent channel removal between the spread's count and copy
operations could leave a default entry, causing a null dereference
during disposal. The listener now uses the dictionary's atomic `Values`
snapshot and disposes those channels outside the listener lock.

Deterministic regressions close zero, one, or all channels at the copy
boundary. Before the fix, the one/all-removal cases reproduced the exact
null dereference; the unchanged-count control passed. All 23
admission/listener cases now pass on net10.0 and net48, and the full
fast Core net10.0 suite passes 5,267 cases with 88 existing platform
skips. Both changed files pass info-severity formatting. A bounded audit
of the formatter's other snapshot rewrites found no further verified
unsafe concurrent-registry copies.

## Shutdown-test observation follow-up at `58403ee8b`

The net48 shutdown-order test raced its own observation: it read
`m_publishWorkerTask` after starting shutdown, which can legitimately
drain the worker and clear that field immediately. The test now captures
the worker before shutdown starts and confirms it is initially running.
It still requires the worker to finish while shutdown remains blocked by
the held manager semaphore. Production behavior is unchanged.

A temporary scheduling perturbation reproduced the exact null-field
failure. The corrected capture passed 100 perturbed iterations; both the
temporary repeat and yield were then removed. The final 126
subscription/lifecycle cases pass on each of net48 and net10.0, and
formatting passes. A complete local Sessions net10.0 run also passed 780
cases with 379 existing capability skips in 7m20s after the earlier
keepalive correction.

## Retirement and alias-startup follow-up at `1a048ff39`

The shadow-generation test now observes its documented asynchronous
cleanup through the existing bounded helper. Removing the replacement
does not join a background drain that already claimed the retired
generation. The test still checks live-registration removal immediately
and requires the old node to disappear; no timeout or production
lifecycle behavior changed. All 81 lifecycle cases pass on each of
net10.0 and net48.

Initial alias-node materialization now retries one timed-out, read-only
snapshot query before creating nodes for that root. The retry is logged
through source-generated logging. Each query retains the original shared
100 ms deadline; repeated timeouts, other errors, and cancellation still
abort startup. Client searches and live refresh do not gain this
immediate retry. Fault-injection tests cover success after one timeout,
a persistent timeout, a different store failure, and cancellation. All
187 related alias/hosting cases and the original certificate-rotation
integration case pass on each target; the positive test records exactly
one retry warning. The alias documentation now accurately describes the
shared request budget.

All changed C# files pass formatter checks, and the complete
54-interface/271-member documentation audit passes. Fresh CI is running
on `1a048ff39`; the previous completed matrix exposed only the two
failures corrected in this batch and their dependent summaries.

## Sample endpoint allocation follow-up at `d08a94925`

The strong-cluster test harness allocated OPC UA and Raft endpoints in
separate released-port batches. Uniqueness was enforced only inside each
batch, so the same OS-assigned port could appear in both protocol
groups. The harness now requests one distinct batch for both groups and
splits it before launching any replicas.

Deterministic one- and three-replica allocation cases reproduced
overlapping groups with the old plan and pass with the corrected plan.
The complete 17-case short-haul sample suite passes without skips; all
failover, historian-continuation, and service assertions remain
unchanged. Both changed files pass info-severity formatting. This
prevents intra-cluster self-collisions but does not claim to reserve
released ports against unrelated external processes.

## Code-review follow-up at `085fd9636`

A review of the complete branch diff found five defects this PR
introduced. All are now fixed, each in its own commit.

`RelatedTo`'s recursive-to-iterative rewrite defaulted the optional
`includeTypeDefinitionSubtypes` and `includeReferenceTypeSubtypes`
operands to `false`. Part 4 §7.7.4 specifies both default to `true`, so
a filter that omitted them silently stopped matching subtypes.
Dynamically created key-credential nodes derived their namespace from
the containing folder, which is the standard `i=18155` node in namespace
0 — the namespace reserved for the OPC UA standard address space; they
now resolve the server-owned namespace and fail with `BadInternalError`
rather than guessing index 1. The retained audience-less
`CreateTokenData` overload produced tokens that verification always
rejects, so it is now `[Obsolete]` and throws instead. A failed
reference to a remote parent left the new node registered with a
one-sided inverse reference, and is now rolled back. An existing `.pfx`
is restricted before it is rewritten, rather than after its new key
bytes are on disk.

One further reported finding was rejected after investigation.
`JwksIssuerKeyResolver` deliberately does not dispose retired key
snapshots: `RetiredKeysAreCollectibleWhileResolverRemainsAliveAsync`
proves the resolver retains no retired snapshot, and
`ReturnedKeysSurviveRepeatedRefreshAndResolverDisposalAsync` requires
already-returned keys to keep verifying across refreshes and resolver
disposal. Adding disposal would have broken that tested contract, so the
exploratory change was reverted.

Validated on net10.0 and net48: 256 Core and 633 Server cases pass, with
one pre-existing net48 skip. The build reports zero compiler and
analyzer diagnostics, all eight included-file info-format checks pass,
the contract audit covers 58 interfaces and 310 members with no
findings, and the 15 bundled migration documents validate. All 122
review threads are resolved; the key-credential namespace thread was
reopened to retract an earlier incorrect answer and then resolved
against the real fix.
## Review-body follow-up at `da0d59cfb`

The re-review at `8daf7f091 + 82dc22b5d` listed four items in its
summary that had no inline thread of their own, so they had not been
picked up. All four are now fixed.

Both event-dispatch permission gates had been tightened from not-Bad to
`IsGood`, which silently dropped events whose permission verdict is
Uncertain; the data-change path in the same file still treated Uncertain
as deliverable, so the two had diverged. `EventSourceRegistry` only
cleared `ConsecutiveFailures` when a source was deactivated, so a source
that had backed off to the 30-second cap kept that delay for the rest of
the process even after starting successfully.
`UserNameIdentityTokenHandler` built a temporary issuer chain whose
`Add` calls take independent certificate references, and never released
them.

The fourth was the receiver-side sequence rollover, which the reviewer
had correctly re-flagged as still open despite its thread being
resolved. `m_sequenceRollover` was set permanently, so the second legal
wrap on a long-lived channel was rejected and faulted the channel along
with the transport-reconnect OPN. OPC 10000-6 does not cap a
SecureChannel at one wrap. The flag is now cleared once an accepted
number has traversed the normal range, which keeps rejecting a replayed
low number that never left the rollover window.

Validated on net10.0 and net48: 636 / 593 Core cases and 135 / 134
Server cases pass, including the new
`UaSCBinaryChannelSequenceRolloverTests` and the Uncertain-permission
delivery cases. The build reports zero compiler and analyzer diagnostics
and the changed files pass included-file info-format checks on both
targets.

The previous head `085fd9636` was fully green: 279 checks passed and
Azure build 18414 succeeded with 137 of 137 jobs, after the platform
automatically retried a job whose NuGet tool download had hit a
transient TLS error.
## Reported issues #4508 and #4509 at `6ad1ecf16`

**#4508 (V2 subscription recreate deadlocks against a faulted transport)
was not yet fixed and is now closed by `bac260a06`.** Both drain sites
the report identifies were still unbounded. A publish worker that
started its request before the quiesce parks inside the service call on
the channel ready gate while holding an active publish request; the
recreate that would make the channel ready is the very operation
awaiting the drain, so the wait never ends.

The drain now cancels in-flight attempts through a per-attempt token
linked to the worker token. An aborted worker rolls its acknowledgements
back, releases the request and returns to the paused park instead of
terminating, so it resumes when the caller leaves the quiesce. Publish
ingress stays quiesced for the caller's operation, so
`DropPendingForSubscription` keeps the guarantee the report asks to
preserve and no blind timeout is added. The new regression parks a
worker on a cancellable gate that never completes on its own: it
reproduces the hang for the full 30-second budget before the change and
passes after it.

**#4509 (identity provider fails the initial connect) was already fixed
for its primary symptom.** `HandleConnectAsync` materializes the
provider's identity through `CreateIdentitySelectionContext` before the
first `CreateAsync`, so Anonymous is no longer substituted against a
UserName-only endpoint. `6ad1ecf16` adds the two remaining details from
the report: the enabled-policy set now includes the negotiated channel
policy, so a client cannot reject the policy it already accepted as
`NotEnabledByClient`; and when discovery has not populated the
endpoint's token policies the connect keeps the configured identity so
the attempt fails at the transport layer for the reconnect policy to
retry, instead of failing at the identity layer.

Validated on net10.0 and net48: 477 / 473 client session and
subscription cases pass, the build is free of compiler and analyzer
diagnostics, and the changed files pass included-file info-format checks
on both targets. The full `Opc.Ua.Sessions.Tests` suite also passes
locally on net10.0 with 782 passed and 379 pre-existing capability
skips.

GitHub checks are green on this head: Build and Test .NET 10.0, Docker
Sample Images CI, and NuGet Publish all succeeded. Azure build 18417
completed 134 of its 135 jobs successfully; its net10.0 Sessions job was
cancelled by the hosted-agent provider ("Received request to
deprovision"), the same infrastructure flake that hit build 18409
earlier on a head predating these changes. That job is not claimed as
passing, and the local full-suite run above covers the same project. The
previous head `da0d59cfb` completed Azure build 18415 with every job
succeeded.
## Review follow-up at `a08f0c12b`

Eleven new review threads arrived, nearly all documentation. Before
acting on each "this is new in 2.0" remark I checked the API against
`master378` rather than assuming, because a genuine 1.5 API still needs
its migration entry.

`ManagedSession`'s server-redundancy refresh had two hard-coded
two-second bounds. The new `ServerRedundancyOptions` exposes
`RefreshTimeout` for the metadata read and `PeerDiscoveryTimeout` for
each peer lookup, defaults both to the previous two seconds, validates
that a bound is positive or `Timeout.InfiniteTimeSpan`, and flows
through `ManagedSessionOptions`, `ManagedSession.CreateAsync`, the
fluent builder, and `DefaultServerRedundancyHandler`.

`AsyncCustomNodeManager` and `KeyCredentialBridgeAuthenticator` are both
absent from `master378`, so their obsolete members had no compatibility
constraint and are removed rather than carried:
`AsyncCustomNodeManager.ReadInitialValue`, the audience-less
`CreateTokenData` that only threw, and the audience-less `CreateProof`
that produced a proof no server accepts.
`CustomNodeManager2.ReadInitialValue` and the `MonitoredNode2` migration
section are genuine 1.5 APIs and are untouched.

The documentation now states master's behaviour instead of the delta
from 1.5.378, with each moved item already covered under
`docs/migrate/2.0.x/`. `IHistorianAnnotationProvider` is described by
its annotation-time key rather than as "legacy" — it is new in 2.0, so
the label implied a compatibility constraint that does not exist. The
KeyCredential bridge payload's `"version"` field is explained as a
vendor-extension format version with no OPC UA counterpart, and its
version-1 upgrade narrative is gone. `CertificateManager.md`'s migration
walkthrough is replaced by the resolver API it targets.
`NodeManagers.md`, `Sessions.md`, `AsyncServerSupport.md`,
`Certificates.md`, and `Profiles.md` no longer announce features
relative to 1.5. `Benchmarks.md`, `WhatsNewIn2.0.md`,
`DeveloperGuide.md`, and `docs/migrate/` keep their 1.5.378 references,
which are their subject.

CI then exposed a defect this batch introduced: five fixtures resolve
`ManagedSession`'s private constructor by its exact positional parameter
types, so the added parameter compiled cleanly but made `GetConstructor`
return `null` at run time. `7df1d5c40` updates every site;
`Redundancy.Client` passes 126 of 126 and the affected client fixtures
112 of 112.

The BouncyCastle question is answered and tracked in #4510:
`Opc.Ua.Core` already project-references `Opc.Ua.Security.Certificates`,
which already carries the conditional BouncyCastle reference, so the
raw-ECDH polyfill belongs there rather than as a new Core dependency.
The `IUserDatabase` metadata question is addressed to another maintainer
and is deliberately left open.

## Reported issue #4509 follow-up at `5d4d13d63`

The reporter verified `6ad1ecf16` and found one case still failing,
correctly. The empty-token-policies branch kept the configured identity
so a server that is down surfaces a transport failure rather than an
identity error. A provider-only configuration has no configured
identity, so it kept `null`: `OpenCoreAsync` turned that into Anonymous
and `OpenValidateIdentity` rejected it with `BadIdentityTokenInvalid`,
silently ignoring the configured provider.

The branch now requires a configured identity to defer to. Without one
the provider is consulted, so its own `BadIdentityTokenRejected` names
the offered and rejected policy set instead. Two regression tests cover
both configurations; the first fails against the previous behaviour by
reaching the session factory with a `null` identity, which is the exact
step that produced the reported error. That branch had shipped without a
test, which is why the case was missed.

## Review and CI status at `5d4d13d63`

**All actionable review threads are resolved: 132 of 133.** The single
open thread is a question the reviewer addressed to another maintainer
about moving user metadata into `IUserDatabase`; it is left open for
them rather than resolved silently. Every resolved thread carries a
reply naming its fix commit.

**Latest-head CI is green on `5d4d13d63`: 279 checks passed, zero
failed, zero pending**, with one conditional signed-package publishing
job skipped. Local validation for this batch: the full
`Opc.Ua.Client.Tests` suite passes 2,430 of 2,430 on net10.0,
`Opc.Ua.Redundancy.Client.Tests` 126 of 126, and `Opc.Ua.Server.Tests`
590 of 590 for the affected selections. Both net10.0 and net48 build
clean for `Opc.Ua.Client`, `Opc.Ua.Server`, and the changed test
projects, and all changed files pass included-file info-severity
formatting on both targets.

The PR remains open; no merge was performed. As stated at the top, this
does not claim that every item in the 244-finding audit backlog is
complete.
## Latest CI follow-up at `b6f2dfa43`

Azure build 18442 exposed a net48 compile regression in the Kafka
adapter: `RuntimeHelpers.GetUninitializedObject` is unavailable on .NET
Framework. Commit `2cf1e0eed` now uses normal `ProducerMessage`
construction on `NETFRAMEWORK` while retaining reflection-based
construction for modern targets that can load a different Dekaf runtime
asset. The exact net48 CI selection passes 92 of 92 tests, and the
netstandard2.1 adapter guard selection passes 11 of 11.

After merging the latest `master`, GitHub Actions run 35643592139 is
green. The initial Ubuntu Core.Schema job built successfully and passed
160 of 160 tests, then hit an external 403 while finalizing its
test-result artifact; its failed-job rerun completed successfully. Azure
build 18451 was canceled when its remote provider deprovisioned the
net10.0 Sessions runner (`Received request to deprovision: The request
was cancelled by the remote provider`). Azure attached no test task log
or source error, and the canceled parent left that child check stale as
in-progress. No source workaround, test skip, or check relaxation was
made for either infrastructure failure.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 68f26810-22b8-476f-abcd-0a8c82d6e208
 2026-09-22 09:10:58 +02:00
Marc Schier da7412c429 Add native xRegistry events and distinct Resource versions (#4396) 
## Summary

- support the xRegistry 0.6.0 distinct Resource/Version hierarchy:
  `Registry -> Group -> Resource -> Versions -> Version`
- instantiate stable logical Resource nodes separately from exact
Version nodes
  while preserving existing Version NodeIds
- delegate logical Resource FileType operations and Version-view
metadata to the
selected default Version with handle pinning and shared writer
reservations
- make Resource and Version delete behavior role-stable and use
`MetaEpoch` /
  `Epoch` respectively
- use stable Resource SourceNodes, exact Version SourceNodes and the
  `Server -> Registry -> Group -> Resource -> Version` notifier chain
- keep a model-metadata-gated client fallback for 0.5 flat Servers; 0.6
Servers
  expose no legacy aliases
- target `HasWoTProjection` at the real stable logical Resource node
- use `AsyncCustomNodeManager` for the registration, fast-path, and
federation managers,
  with awaited lifecycle, node mutations, event delivery, and cleanup
- handle startup rollback, reentrant deletion callbacks, undisclosed
file handles,
  and retryable cleanup without blocking unrelated file operations

## Authoritative dependencies

- companion specification: OPCF-Members/spec-drafts#32 at
  `d3399fca025025a73e5a6b599f850a81d985c060`
- OPC UA binding: xregistry/spec#568
- xRegistry event semantics: xregistry/spec#563, merged as
  `66ee31e47fbadff6ab982ae1516f7d9065e46b64`
- companion NodeSet SHA-256:
  `369f5f3f1814b333e4eeb52bfb4b6a730cdff372505a5f91a54c94f0016ac4cf`

## Review feedback

- keep the WoT Connectivity information-model designation at `1.1`;
draft revisions update the publication date without changing the model
version
- eliminate all 198 unique CA1861 diagnostics across 54 files and remove
seven obsolete project suppressions
- fix the analyzer-clean touched-path CA1307 and the hierarchy follow-up
CA1846/CA1851 diagnostics without dropping legacy-TFM compatibility

- recover the reconcile queue after callback faults/cancellation and
continue accepted pending changes
- remove migration guidance for the xRegistry managers, which are new in
2.0

## Compatibility

- public WoT Connectivity model designation remains `1.1`
- generated event-record ABI, constructors, options, NodeIds, and stored
documents remain compatible
- the generic xRegistry managers are new in 2.0 and implement
`IAsyncNodeManager`;
use `IAsyncNodeManagerFactory` and the async lifecycle hooks (see
`docs\XRegistry.md`)
- persistence schema and document blobs remain compatible unless noted
below

## Validation

- merge validation at `980422f76` with upstream `e8bec27d5`: 454
targeted WoT/materialization/model/registry cases and all 327 XRegistry
cases passed on both net10.0 and net48; all 29 net10 aggregation sample
cases passed
- feedback regressions at `308e8ba74`: 55 targeted queue and live
registry event/projection/lifecycle cases passed on each of net10.0 and
net48
- prior CI: all 266 checks passed at `6489a0908`; the unchanged Ubuntu
Core certificate-counter test and dependent jobs passed on the one-time
retry ([run 34117304035, attempt
2](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/34117304035/attempts/2))
- `Opc.Ua.XRegistry.Tests` — 327 passed on net10.0 and net48 for
migration commit `32c914aaf`
- `Opc.Ua.WotCon.Tests` net10.0 — 1348 passed
- `Opc.Ua.WotCon.Bindings.Tests` net10.0 — 683 passed
- `Opc.Ua.SourceGeneration.Core.Tests` net10.0 — 3807 passed, 8 skipped
- `Opc.Ua.SourceGeneration.Tests` net10.0 — 145 passed
- `Opc.Ua.SourceGeneration.Stack.Tests` net10.0 — 94 passed
- affected XRegistry and WoT client/server projects build successfully
- xRegistry NodeSet is byte-identical to the companion artifact
- repository-wide CA1861 inventory — 198 unique diagnostics across 54
files reduced to zero
- targeted changed-fixture validation — 1,098 passed, 3 skipped, 0
failed
- constrained full `UA.slnx` net10.0 Release build — CA1861 = 0
- `Opc.Ua.XRegistry.Client` net472 and netstandard2.1 builds — 0
warnings, 0 errors
- independent shared-array safety review — no significant issues
- `git diff --check`

The final review regressions cover concurrent reconcile/open, synthetic
logical
handle collisions across default switches, cross-session Close
isolation,
shared writer reservations, logical FileType property/change
propagation,
logical Labels delegation, role-fixed deletion, event sources,
model-version
gating, and stable `HasWoTProjection` targets.

The companion specification and protocol binding are reviewed in the
linked PRs above.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5c79a1ed-6dab-4522-a9fc-6548caa78ad5
Copilot-Session: b605797f-a97a-4cd0-8f12-016d33c2fd88
Copilot-Session: 7ef369a5-2307-4f71-8a11-15af0e72f240
Copilot-Session: e716d455-8397-4d22-9c5a-398847ab7317
 2026-09-08 04:46:40 +00:00
romanett 52df519e50 Expose the client configuration provider and add an opt-in eager load on host start (#4343) 
# Description

`AddClient(configurationFile)` (#4328) loads the supplied configuration
XML lazily on the first DI session connect, and the
`ClientSuppliedConfigurationProvider` behind it was reachable only
through internals. Hosted clients whose user interface needs the
`ApplicationConfiguration` **before any session exists** therefore had
no public way to trigger the load or to read the configuration back —
the concrete blocker for moving the 14 WinForms sample clients and the
GDS client off the classic eager `ApplicationInstance` path
(OPCFoundation/UA-.NETStandard-Samples#794 /
OPCFoundation/UA-.NETStandard-Samples#808).

Two small additions, exactly as designed in #4340:

**1. `OpcUaClientOptions.ConfigurationProvider` is now public** (`get`;
the setter stays `internal`).

It is already typed as the public
`IOpcUaApplicationConfigurationProvider`, so consumers get everything
they need with **no new types**: `GetAsync(ct)` as an explicit load
trigger that does not connect a session, `Configuration` for the loaded
document, and `Application` (`IApplicationInstance`) for the
applications that manage their own instance certificate. Registering the
supplied provider as `IOpcUaApplicationConfigurationProvider` in DI was
deliberately *not* done — `OpcUaServerHostedService` binds to the last
registered interface when it has no document of its own, so in a
combined client+server host a client-only document would silently
capture the server.

**2. `OpcUaClientOptions.LoadConfigurationOnStart` — an opt-in eager
load for hosted applications.**

It registers an internal `ClientConfigurationLoaderHostedService` that
awaits `GetAsync` during host start, the client-side twin of
`ReverseConnectManagerHostedService`. When `host.StartAsync()` returns,
the document is loaded and validated, the application-instance
certificate is ensured, `Configuration` is published onto the resolved
options (exactly as the connect path does), and a load failure fails the
host start instead of surfacing on the first connect. It is registered
ahead of the reverse-connect hosted service, so the configuration is
complete before anything consumes it.

The flag applies to every provider source — a document supplied via
`ConfigurationFile` / `ConfigurationStream` and a shared
`ConfigureApplication(...)` provider alike — and is a no-op for an
explicit `Configuration`, which is already complete. It is bindable from
`OpcUa:Client:LoadConfigurationOnStart`. Applications without a Generic
Host call `GetAsync` on the provider directly instead; both routes share
the provider's single-flight load, so the configuration is never loaded
twice.

A WinForms client then becomes:

```csharp
services.AddOpcUa().AddClient("MyClient.Config.xml", o => o.LoadConfigurationOnStart = true);

await host.StartAsync();

ApplicationConfiguration configuration = host.Services
    .GetRequiredService<OpcUaClientOptions>()
    .ConfigurationProvider!.Configuration;
```

**Reviewer note:** because the flag decides a *service registration*, it
must be set inside the `AddClient(...)` callback or bound from the
configuration section — setting it on the resolved options afterwards
has no effect. This is called out in the XML docs and in
`DependencyInjection.md`.

**Coverage:** three tests added to `ClientConfigurationFileTests` for
the supplied-document path (eager load on start including the
certificate bootstrap and `Application` surface, load failure surfacing
on start, no hosted service registered without the flag) and a new
`ClientEagerConfigurationLoadTests` fixture for the provider-agnostic
behavior (shared `ConfigureApplication(...)` provider awaited exactly
once, explicit-`Configuration` no-op, host-startup-token cancellation
aborting the load, configuration-section binding, and an end-to-end
`host.StartAsync()` test). All 331 `ClientBuilder` tests pass (8 new),
as do the 185 server hosting tests including the combined client+server
fixtures. `Opc.Ua.Client` builds warning-free on all target frameworks;
the test project builds clean on net10.0 and net472.

**Docs:** new "Client: loading the configuration eagerly" section in
`docs/DependencyInjection.md` and a pointer in the `Opc.Ua.Client` NuGet
README.

With these in place the sample clients can switch in a follow-up to
OPCFoundation/UA-.NETStandard-Samples#808.

## Related Issues

- Fixes #4340
- Builds on #4328

## Checklist

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

- [x] 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.
- [ ] I have addressed **all** PR feedback received.

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

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
 2026-08-30 20:50:46 +02:00
romanett 1f0125c385 Fix reverse connect hold time when several Servers share one listener (#4342) 
# Description

Fixes the reverse connect problem reported in #3985 on `master`. #4009
and #4059 addressed the listener startup diagnostics and the async
lifecycle, but the actual hold time defect described in [mrsuciu's
comment on the
issue](https://github.com/OPCFoundation/UA-.NETStandard/issues/3985)
survived both refactors unchanged.

Symptom: a Client which uses one reverse connect listener port for
several Servers connects to the first Server, and every following
`WaitForConnectionAsync` times out. A separate listener port per Server
was the only workaround.

## Root cause

- Servers which send `ReverseHello` before the application registered a
waiting connection for them are held in `OnConnectionWaitingAsync` for
`HoldTime`.
- All held connections share a single `CancellationTokenSource`, so
`RegisterWaitingConnection`/`WaitForConnectionAsync` wakes up **every**
held connection, not only the one it was registered for.
- The hold loop was `while (!matched) { ... break; }` with an
unconditional `break`, so each woken connection got exactly one re-match
attempt. The connections which did not match the new registration fell
out of the loop unaccepted, although most of their own hold time was
left.
- An unaccepted connection is force faulted by the listener
(`TcpReverseConnectChannel`, "The reverse connection was rejected by the
client"), so that Server is dropped and only returns on its next
`ReverseHello` interval, long after the Client timed out waiting for it.

Separate ports avoid the shared wakeup path, which is exactly why that
workaround succeeds.

## Changes

- `OnConnectionWaitingAsync` re-arms the hold after a wakeup: it
re-matches, and if the registration which caused the wakeup was for
another Server the connection keeps waiting for the remainder of its own
hold time. The loop is still bounded by `HoldTime`, so a Server which is
never registered for is rejected exactly as before.
- New `IsCallbackDrainPending()` check before the hold is re-armed.
`DrainConnectionCallbacksAsync` cancels the hold token without renewing
it, so a re-armed hold would spin on the already cancelled token until
the hold time expires and would block the drain until then. With the
check a held callback releases its transport immediately, which keeps
the existing `StopDrainsHeldConnectionCallbackBeforeListenerClose` and
`DisposeDrainsHeldConnectionCallbackBeforeListenerDisposal` guarantees
intact.
- `docs/ReverseConnect.md`: document the `HoldTime` semantics in the
"Sharing a listener across multiple Servers" section.

No public API change.

## Validation

- New `HeldConnectionSurvivesRegistrationForAnotherServer` test in
`ReverseConnectManagerLifecycleTests` drives two held callbacks through
`InvokeConnectionWaitingForTest` and proves the second Server keeps its
connection when the first Server is registered. Verified that it fails
against the previous behaviour with "the reverse connection of the
second Server was released before its hold time expired".
- `FullyQualifiedName~ReverseConnect` (183 tests, including
`ReverseConnectManagerTests`, `ReverseConnectManagerLifecycleTests` and
`ReverseConnectHostTests`): all pass on net10.0 and net48.

## Related Issues

- Relates to #3985.
- The same fix for the LTS branch is #4341 (base `master378`).

## Checklist

- [x] 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.
- [ ] I have addressed **all** PR feedback received.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-30 16:38:07 +00:00
romanett 70c8ce4649 Allow passing an existing config XML to the client DI AddClient (#4328) 
# Description

> **Stacked PR**: based on `romanett/server-di-config-xml-e30c4f`
(#4325) so the diff shows only the client changes. Once #4325 merges
this will be rebased onto `master`.

Gives the **client** DI surface the same migration path for existing
configuration XML files that #4325 gave the server: applications that
already own a classic `MyClient.Config.xml` can pass it to `AddClient`
and keep **all** of their settings.

New API surface:

- `AddClient(string configurationFile, Action<OpcUaClientOptions>?
configure = null)` loads the `ApplicationConfiguration` from the file;
the optional callback configures the remaining client options (session
endpoint, identity, reverse connect, ...).
- `AddClient(Stream configurationStream, ...)` for documents that are
not files on disk, e.g. embedded resources. The stream is read once and
disposed after loading.
- `OpcUaClientOptions.ConfigurationFile` / `ConfigurationStream` back
the overloads, so the options-callback route works too and the file path
can be bound from configuration (`OpcUa:Client:ConfigurationFile`).
- `OpcUaClientOptions.ConfigureLoadedConfiguration` runs after the
document is loaded and validated but before the application-instance
certificate check, for code-side overrides of individual file settings.

Mechanism and behavior:

- A new internal `ClientSuppliedConfigurationProvider` implements
`IOpcUaApplicationConfigurationProvider` and plugs into the client's
existing configuration-provider seam. The document loads **lazily on
first use** — the first session connect, reverse-connect startup, or an
explicit `GetAsync` — through
`ApplicationInstance.LoadApplicationConfigurationAsync`, so every
setting (security configuration, certificate stores, transport quotas,
client configuration) applies exactly as on the classic path, including
localhost fix-up.
- The load **ensures the application-instance certificate** (mirroring
the shared `ConfigureApplication(...)` provider), which the pre-existing
manual workaround — pre-loading and assigning
`OpcUaClientOptions.Configuration` — never received. DI-registered
`ICertificateManager` / `ICertificatePasswordProvider` are honored.
Single-flight loading; `IAsyncDisposable` like the shared provider.
- The supplied document is authoritative and takes precedence over a
shared application registered with `ConfigureApplication(...)`.
Ambiguous combinations are rejected eagerly at `AddClient` with clear
`InvalidOperationException`s: file + stream, document + explicit
`Configuration`, document + application identity properties, and a
white-space path arriving through configuration binding. The start-time
`IValidateOptions` validator accepts a supplied document as a
configuration source.
- `ConnectManagedSessionAsync` publishes the lazily loaded configuration
onto the resolved options after `GetAsync`, so the channel manager and
session builder see it exactly as on the other configuration paths;
reverse connect consumes the provider through its existing async-startup
path unchanged.

Coverage: new `ClientConfigurationFileTests` fixture (8 tests) — loading
from file and stream with assertions on the document settings, the
override callback, the certificate bootstrap, provider caching and lazy
semantics, precedence over `ConfigureApplication`, load failure for a
missing file, argument validation, ambiguous-combination rejection,
configuration-section binding, and validator acceptance. All 323
existing ClientBuilder tests and all 180 server hosting tests (including
the combined client+server fixtures) pass; `Opc.Ua.Client` builds
warning-free on net472, net8.0, and net10.0 (AOT analyzers).

Docs: new "Client: using an existing configuration XML file" section in
`docs/DependencyInjection.md` with cross-links from the server section,
an updated pointer in the 2.0 migration guide, and a NuGet README
mention.

## Related Issues

- Fixes #4327
- Stacked on #4325 (server-side counterpart)

## Checklist

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

- [x] 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.
- [ ] I have addressed **all** PR feedback received.

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-08-29 13:33:40 +00:00
romanett c57f31e023 Fix three flaky tests (Robotics, Client, Di) (#4289) 
## Summary
Fixes the three flaky tests surfaced in the last full nightly (ADO build
17152, commit `aad8350`, run
[95295970000](https://github.com/OPCFoundation/UA-.NETStandard/runs/95295970000)).
Each failure has a cross‑platform/cross‑TFM signature (fails on multiple
legs), confirming a race/timing issue rather than a platform bug. All
changes are **test-only** — no production behavior changes.

## Fixes

### Robotics `TheQueueIsBoundedByMaxQueueDepth` (#4286)
The executor gate blocks execution *after* an item is dequeued, and
`MaxQueueDepth` counts only queued items. The background dispatch pump
could move `"a"` out of the queue into the executing slot before `"b"`
was submitted, so the queue wasn't full and `"b"` was admitted. Fix:
`Pause` queue dispatch before submitting (admission still runs) so `"a"`
stays queued and the bound is enforced deterministically.

### Client `DisposeDrainsHeldConnectionCallbackBeforeListenerDisposal`
(#4287)
The drain ordering in production is correct, but the test asserted
`callback.IsCompleted`, which flips one async continuation *after* the
drain signal fires — so the close gate can be entered before the
returned Task transitions to completed. Fix: assert on
`ActiveConnectionCallbackCountForTest == 0` (`Is.Zero`), the
deterministic invariant the drain actually waits on.

### Di `LiveValuesFlowThroughConnectorToUsdSinkAsync` (#4288)
Used a fixed `Task.Delay(4000)` before asserting sink writes; slow mac
agents didn't deliver in time. Fix: poll `WasWritten(...)` up to 30s
(fast on healthy runs, robust on slow agents), then stop.

## Validation
Built and ran each affected test on `net10.0` — all pass:
- `Opc.Ua.Robotics.Tests` — 1/1 passed
- `Opc.Ua.Client.Tests` — 1/1 passed
- `Opc.Ua.Di.Tests` — 1/1 passed

Closes #4286
Closes #4287
Closes #4288

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-19 18:14:53 +00:00
Copilot a7d11db1bd Enable fluent subscription option configuration (#4248) 
# Description

`AddSubscriptions(Action<SubscriptionOptions>)` could not configure
init-only properties. Subscription options are now mutable for DI
configuration delegates.

- **Configuration**
- Expose writable subscription option properties while retaining
record-based `with` support.

```csharp
.AddSubscriptions(options =>
{
    options.PublishingEnabled = true;
    options.SendInitialValuesOnTransfer = true;
})
```

- **Coverage**
- Verify configured values are retained by the resolved subscription
options monitor.

## Related Issues

## 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.
- [ ] 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.

<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes #4245

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-14 17:40:48 +00:00
Marc Schier 2a4bc0fe24 Make reverse connect lifecycle fully asynchronous (#4059) 
## Summary

- add cancellation-aware asynchronous start, lazy initialization, stop,
reload, and disposal for `ReverseConnectManager`
- make listener replacement transactional with explicit ownership,
rollback/restoration, stale watcher protection, and deterministic
concurrent start/stop/dispose behavior
- add injectable async configuration providers, remove the former
protected configuration hooks, and retain obsolete synchronous lifecycle
compatibility wrappers
- integrate eager Generic Host startup plus non-host lazy startup,
including custom application-configuration providers
- serialize `ReverseConnectHost` lifecycle and extend raw TCP, Kestrel
TCP, WSS, hosting, concurrency, rollback, and NativeAOT coverage
- document the async lifecycle, hosting behavior, cancellation
guarantees, provider model, and migration path

## Validation

- `Opc.Ua.Client` Release build across all supported target frameworks
- reverse-connect client and core tests on net10.0 and net48
- raw TCP, Kestrel TCP, and WSS integration tests on net10.0; raw TCP on
net48
- NativeAOT publish and 115-test executable run

Closes #4013.

Related: #3985, #4009.
 2026-07-23 10:40:32 +02:00
Copilot f95d65754d Register mandatory opc.tcp transport in AddOpcUa (make AddOpcTcpTransport optional) (#4074) 
# Description

`AddOpcUa()` did not register an `ITransportBindingRegistry`. The client
resolves it with `GetRequiredService(...)`, so a client built with
`AddOpcUa().AddClient(...)` threw during managed-session creation unless
`AddOpcTcpTransport()` was also called. (Servers tolerated a missing
registry via `ServerBase`'s lazy `WithDefaultTcp()` fallback, which is
why `MinimalBoilerServer` worked but `MinimalClient` did not.)

This change makes the mandatory `opc.tcp` transport always available
while keeping it overridable and removable, then reverts the sample
workaround.

## Changes

- **`AddOpcUa()` seeds the mandatory transport.**
`AddTransportBindingRegistry()` now seeds the registry with the
raw-socket `opc.tcp` secure-channel listener and tcp connection channel
factories **before** running the registered
`ITransportBindingConfigurator`s, and `AddOpcUa()` calls it.
`AddOpcTcpTransport()` is now optional (kept for backward
compatibility).
- **Overridable.** Because the seed runs before configurators,
`AddKestrelOpcTcpTransport()` / `AddHttpsTransport()` /
`AddWssTransport()` / `AddCustomTransport<,>()` still override the
seeded defaults last-writer-wins per URI scheme — now without needing
`AddOpcTcpTransport()` first.
- **Removable.** Added `RemoveListenerFactory(scheme)` /
`RemoveChannelFactory(scheme)` to `ITransportBindingRegistry` (+
`DefaultTransportBindingRegistry`) so an implementer can unregister the
seeded defaults from a plain `ITransportBindingConfigurator` — no fluent
API required.
- **Sample.** Reverted the `MinimalClient` `.AddOpcTcpTransport()`
workaround; it now connects with just `.AddOpcUa().AddClient(...)`.
- Small note in `docs/Transports.md` that `AddOpcTcpTransport()` is
optional.

## Tests / validation

- New/updated unit tests in `TransportBindingRegistryTests`:
mandatory-`opc.tcp`-by-default,
override-default-without-`AddOpcTcpTransport`,
implementer-can-unregister-default, and `Remove*` unit tests; updated
`ParallelServiceProvidersHaveIndependentRegistries` for the seeded
default.
- Client DI regression test in `ClientDiFluentApiTests`
(`AddOpcUa().AddClient(...)` resolves the registry + channel manager
without an explicit tcp transport).
- Updated the `SpyTransportBindingRegistry` test double for the two new
interface members.
- **End-to-end:** started `MinimalBoilerServer` and ran `MinimalClient
--auto-accept` → discovered a `SignAndEncrypt` / `Basic256Sha256`
endpoint, connected, browsed the Objects folder, read
`ServerStatus.CurrentTime`, and created a subscription (exit code 0)
with no `AddOpcTcpTransport()`.
- **Green locally:** `Opc.Ua.Core.Tests` (4123 passed),
`Opc.Ua.Client.Tests` (1962 passed), `Opc.Ua.Server.Tests` Hosting (141
passed) on **net10.0**; transport + client-DI tests on **net48**;
`Opc.Ua.Core` builds with 0 warnings.

## Related Issues

Tracked by the linked issue.

## 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.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@hotmail.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
 2026-07-23 07:26:19 +02:00
Copilot b76022d13d Add MinimalClient - lightweight OPC UA console client with fluent API (#4006) 
# Description

Adds `MinimalClient`, a compact console OPC UA application demonstrating
the unified dependency-injection surface, secure endpoint discovery,
managed sessions, fluent V2 subscriptions, and Alarms & Conditions
support.

## Changes

- Adds `ConfigureApplication(options => ...)` to build and validate one
shared `ApplicationConfiguration` for combined client/server hosts.
- Extends `OpcUaClientOptions` so the existing `AddClient(options =>
...)` callback accepts application identity, PKI, and
certificate-validation settings like `AddServer(...)`.
- Composes root `ConfigureApplication(...)` values with client defaults
in either registration order; shared values take precedence and client
values fill gaps.
- Keeps server samples on their established `AddServer(...)`
configuration pattern.
- Adds `MinimalClient` as a .NET 10 Native AOT console application using
top-level statements and `HostApplicationBuilder`.
- Discovers a `SignAndEncrypt` / `Basic256Sha256` endpoint by default;
`--insecure` and `--auto-accept` are explicit opt-ins with warnings.
- Demonstrates fluent `AddSubscription(...)`,
`TryAddMonitoredItem(...)`, and `AlarmClientFactory` usage.
- Adds targeted configuration, client, and combined client/server tests.

## Example

```csharp
builder.Services
    .AddOpcUa()
    .AddClient(options =>
    {
        options.ApplicationName = "MinimalClient";
        options.ApplicationUri = "urn:localhost:OPCFoundation:MinimalClient";
        options.ProductUri = "uri:opcfoundation.org:MinimalClient";
        options.Session = new ManagedSessionOptions
        {
            SessionName = "MinimalClient",
            SessionTimeout = TimeSpan.FromSeconds(60)
        };
    })
    .AddDiscoveryAndConnect(options =>
    {
        options.DiscoveryUrl = discoveryUrl;
        options.SecurityMode = MessageSecurityMode.SignAndEncrypt;
        options.SecurityPolicyUri = SecurityPolicies.Basic256Sha256;
    })
    .AddSubscriptions()
    .AddAlarms();
```

## Related Issues

_If there is no issue yet, open one and link it here._

## 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.
- [ ] I fixed all failing and flaky tests in the CI pipelines and all
CodeQL warnings.
- [x] I have addressed all PR feedback received.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <romanett98@gmail.com>
Co-authored-by: Marc Schier <marcschier@hotmail.com>
 2026-07-17 19:36:53 +02:00
Marc Schier 41a648c9db Fail fast on reverse connect listener startup (#4009) 
# Description

Fail reverse-connect listener startup immediately instead of logging
bind failures and surfacing a later connection timeout.

- Validate and stage configured and programmatic listener endpoints
before replacing active configuration.
- Open all listeners atomically and report endpoint-specific failures
with `BadTcpEndpointUrlInvalid`, the transport-specific status, or
`BadNoCommunication`.
- Roll back listeners opened by a failed attempt without reacquiring the
manager lock, including forced listener disposal when normal close
fails.
- Preserve meaningful `ServiceResultException` status codes through both
`StartService` overloads.
- Dispose replaced configuration watchers and reject callbacks from
stale watcher generations.
- Keep the intended shared-listener behavior: one long-lived
`ReverseConnectManager` can accept connections from multiple Servers on
the same Client URL.
- Expand `Docs/ReverseConnect.md` with listener lifetime, shared-manager
usage, diagnostics, and specification references.

## Validation

- `ReverseConnectManagerTests`: net10.0 and net48.
- `ReverseConnectHostTests`: net10.0 and net48.
- Ordered reverse-connect listener reuse integration tests: net10.0.

## Related Issues

- Relates to #3985.

## 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.
- [ ] I have addressed **all** PR feedback received.
 2026-07-17 13:20:24 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00