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

28 Commits

Author SHA1 Message Date
romanett d552634258 Enable ECC security policies on net48 via a BouncyCastle raw-ECDH polyfill (#4703) 
Closes #4510.

## Summary

OPC UA Part 6 feeds the **raw** ECDH shared secret into HKDF. .NET
Framework only exposes `ECDiffieHellman.DeriveKeyMaterial`, which always
hashes it, so the ECC SecureChannel and user-token policies were
unavailable on `net48` (`SecurityPolicies.GetInfo` returned `null`).

This adds a BouncyCastle polyfill for just that primitive:

-
**`Opc.Ua.Security.Certificates/Org.BouncyCastle/EcdhRawAgreement.cs`**
(`net48` only): an internal `DeriveRawSecretAgreement(this
ECDiffieHellman, ECDiffieHellmanPublicKey)` extension using
`ECDHBasicAgreement`. It returns the X coordinate padded to the field
size, exactly like the .NET 8 BCL method. Keys stay CNG keys; ECDSA
signatures and HKDF stay on the platform — only the agreement step is
managed code. The point is validated (on curve, not infinity) and
mismatched curves are rejected.
- Because the extension mirrors the .NET 8 signature,
`Nonce.GenerateSecret` now has one code path on every target.
- `SupportsRawEccSecretAgreement()` is removed; the CBC-based ECC
policies (`ECC_nistP256`, `ECC_nistP384`, `ECC_brainpoolP256r1`,
`ECC_brainpoolP384r1`) are offered on every target (still subject to OS
curve support).
- BouncyCastle was already a `net48` dependency of
`Opc.Ua.Security.Certificates`; no new package. (Only `net48` is
downlevel now — `netstandard2.1`/`net472` from the issue text are no
longer targets.)

**Not in scope:** the `_AesGcm` / `_ChaChaPoly` ECC variants (and PubSub
DTLS) still need .NET 8 because .NET Framework has no AEAD ciphers —
follow-up stacked on this branch.

## Docs

`EccProfiles.md` (Known limitations), `migrate/2.0.x/certificates.md`,
`ProductionChecklist.md`, `Profiles.md`, `WhatsNewIn2.0.md`. Calls out
that on `net48` the agreement step does not run in a FIPS/validated
module.

## Tests (local)

| Suite | Result |
|---|---|
| `Opc.Ua.Core.Tests` full, net48 | 5956 passed, 0 failed |
| `EccInteropTests` net48 vs 1.5.378 peer (net8.0, native raw ECDH) | 26
passed; 28 skipped (AEAD + curve25519/448) — all four CBC curves, both
directions, UserName tokens, token renewal |
| `Opc.Ua.Sessions.Tests` ECC filter, net48 | 52 passed, 0 failed |
| Server `SessionCoverageTests`/`EphemeralNonce*`, Configuration builder
tests, net48 | all pass |
| Nonce/ECC/EncryptedSecret + session tests, net10.0 | all pass |

New tests: symmetric agreement on all four curves; on net48 an oracle
against CNG (`SHA256(raw) == DeriveKeyFromHash`); curve mismatch and
off-curve rejection. Tests that asserted the old net48 limitation were
updated.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 21:52:44 +02:00
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
Klaus Landsdorf 2a619cdf05 Client: never select an OpenAPI endpoint in SelectEndpoint, and keep a closed Session from sending requests without authenticationToken (#4646) 
# Description

Two client fixes, one commit each.

**1. `CoreClientUtils.SelectEndpoint` skips OpenAPI endpoints.**

- An HTTPS listener with the REST binding announces its OpenAPI endpoint
with the same URL and message mode as its binary mode-None endpoint.
`SelectEndpoint` matched on the URL scheme and the message mode only, so
the first of the two in the GetEndpoints result won, in the main pass
and in the HTTPS fallback for `useSecurity`.
- The OpenAPI mapping (OPC 10000-6 §G.3) is the REST binding, not the
binary transport the caller connects with. With the default channel
bindings no channel factory is registered for the OpenAPI profile
(`ClientChannelManager` maps it to `opc.https+webapi`), so a Session
cannot be created on it; with the REST channel registered, the Session
silently runs over REST.
- Following OPC 10000-4 §5.5.4 (select by `TransportProfileUri`), both
passes now skip endpoints with the HTTPS or WSS OpenAPI transport
profile. Endpoints without a `TransportProfileUri` are selected as
before.

**2. A closed Session does not send requests without
authenticationToken.**

- A request with a null `authenticationToken` is a session-less
invocation (OPC 10000-4 §6.3.1).
- After `CloseAsync(closeChannel: false)`, a request sent through the
same `Session` object went out that way. A server that supports
session-less invocation processed it as one instead of rejecting it.
- `Session` now records that a session was created (`SessionCreated`
with a non-null cookie; `OpenAsync` now calls it through the override
instead of `base.SessionCreated`). From then on `UpdateRequestHeader`
fails a request without token locally with `Bad_SessionIdInvalid`, and
the request is never sent. CreateSession and ActivateSession are exempt,
so reopening and reactivating keep working. A Session that was never
created behaves as before.

## Changes

- `src/Opc.Ua.Client/CoreClientUtils.cs`: `IsOpenApiEndpoint`, used in
both selection passes; the `<returns>` documentation says that an
OpenAPI endpoint is never returned.
- `src/Opc.Ua.Client/Session/Session.cs`: overrides of `SessionCreated`
and `UpdateRequestHeader` (both virtual in `SessionClient` /
`ClientBase`), one `volatile bool` field.

## Tests

- **New, part 1** (`Opc.Ua.Client.Tests`,
`CoreClientUtilsSelectEndpointTests`):
`SelectEndpointSkipsTheOpenApiEndpointListedFirst`,
`SelectEndpointWithSecurityFallbackSkipsTheOpenApiEndpoint`,
`SelectEndpointSkipsTheWssOpenApiEndpoint`,
`SelectEndpointReturnsNullWhenOnlyOpenApiEndpointsMatch` (with and
without `useSecurity`),
`SelectEndpointKeepsTheFirstBinaryEndpointWithoutTransportProfile`.
- **New, part 2** (`Opc.Ua.Client.Tests`, `SessionTests` with the
channel mock):
`RequestAfterCloseFailsLocallyWithBadSessionIdInvalidAsync` (the Read
never reaches the channel), `ActivateSessionIsStillSentAfterCloseAsync`,
`RequestAfterANewSessionIsCreatedCarriesItsTokenAsync`,
`RequestBeforeASessionIsCreatedIsNotRefusedAsync`.
- **Updated:** `Opc.Ua.Sessions.Tests`
`ClientTest.ReconnectSessionOnAlternateChannelAsync` closes the Session
and then the channel. The read that follows now fails with
`Bad_SessionIdInvalid` before it reaches the closed channel, and the
test expects that code instead of the codes of the closed channel (on
master it gets `Bad_NotConnected`).
- **Fail without the fix:** with `CoreClientUtils.cs` and `Session.cs`
from master, all five SelectEndpoint tests that involve an OpenAPI
endpoint (6 cases) fail, as does
`RequestAfterCloseFailsLocallyWithBadSessionIdInvalidAsync`, and
`ReconnectSessionOnAlternateChannelAsync` fails with `Bad_NotConnected`.
`SelectEndpointKeepsTheFirstBinaryEndpointWithoutTransportProfile` and
the three other Session tests pass there by design: they pin behaviour
that must not change.
- `Opc.Ua.Client.Tests` 2743/2743 and `Opc.Ua.Sessions.Tests` 672 passed
/ 287 skipped / 0 failed on net10.0 and net8.0 (macOS).
- `Opc.Ua.Client` builds for net48, net8.0, net9.0 and net10.0 with 0
warnings; `dotnet format` (whitespace, style, analyzers) reports nothing
on the changed lines; changed lines covered 26/26 (100 %) with the two
suites above.
- Fork CI (full matrix incl. macOS) and CodeQL: [CI run
37128535726](https://github.com/biancode/UA-.NETStandard/actions/runs/37128535726)
(154 jobs) and [CodeQL run
37128537502](https://github.com/biancode/UA-.NETStandard/actions/runs/37128537502).
CodeQL is green; CI is green except `Opc.Ua.Interop.Tests`
`LegacyClientExtendedCheckAsync("ComplexTypes")` /
`LoadAndDecodeLegacyComplexTypesAsync` ("1 of N structures were not
decoded"), which came with #4630 and fails the same way on other open
pull requests; this change does not touch ComplexTypes.

## Notes for review

- Part 2 changes the code a caller sees for a request on a closed
Session: `Bad_SessionIdInvalid` is raised locally instead of whatever
the server or the closed channel answers. Reconnect paths are not
affected, because they use ActivateSession or a new Session object. If
you prefer to keep part 2 out, its commit can be dropped without
affecting part 1.
- The test framework's own endpoint selection (`ClientFixture`) is left
as it is; the upstream test servers do not need the change.

## Related Issues

- Fixes #4645

## 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. (Affected
suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered
by the fork CI's Windows net48 legs.)
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
 2026-10-04 12:28:26 +02:00
romanett 61edaa1ed0 Remove netstandard2.1 and net472 target frameworks (#4624) 
# Description

Drops support for **netstandard2.1** and **.NET Framework 4.7.2**.
Libraries now target `net48`, `net8.0`, `net9.0` and `net10.0`.
`Opc.Ua.Types` and `Opc.Ua.SourceGeneration.Core` still also target
`netstandard2.0`, but only because the Roslyn source generators load
them. There is no general netstandard2.0 support.

**Preprocessor directives** (112 files)
- Every `#if`/`#elif` mentioning `NETSTANDARD2_1*`, `NET472*`,
`NET_STANDARD_TESTS`, `SKIP_ECC_CERTIFICATE_REQUEST_SIGNING` or
`NETSTANDARD*` was evaluated against the remaining TFM set (net48,
net8/9/10, plus netstandard2.0 inside `Opc.Ua.Types`/`tools`) and
simplified. Dead branches were removed.
- Example: `NETSTANDARD2_1 || NET8_0_OR_GREATER` → `NET8_0_OR_GREATER`.
- Example: `NET8_0_OR_GREATER || NET472_OR_GREATER` → removed (always
true).
- Netstandard-only code was removed: `UnixFileModeShim`, the CRDT
reflection path in `ReplicatedGossipOptions.CreateReaderOptions`, and
the netstandard2.1-only test variants.

**Custom test logic**
- `targets.props` drops the `net472`, `netstandard2.0` and
`netstandard2.1` `CustomTestTarget` profiles, `NetStandardTests` and
`SKIP_ECC_CERTIFICATE_REQUEST_SIGNING`. Supported values are now
`net48`, `net8.0`, `net9.0`, `net10.0`.
- The `NET_STANDARD_TESTS` define is removed from 29 test projects.
- The "build netstandard shells as net10.0" overrides are removed from
18 projects.
- `RestrictForLegacyTfm` now only applies to `net48`.
- `tests/customtest.bat` is updated to the new values.

**Build / packaging / CI**
- MSBuild TFM conditions are simplified: Kestrel package choice, MQTTnet
v4 (net48 only), BouncyCastle, Di/OpenUsd/Vision/Stress test excludes,
and `System.IO.FileSystem.AccessControl` (dropped from Core).
- The MigrationAnalyzer nuspec drops its net472/netstandard2.1 lib
groups. The Azure signing lists are updated.
- `get-ci-matrix.ps1` removes the net472/netstandard test profiles
(including the PR-gating `linux-netstandard2.1`) and the matching build
legs.

**Breaking API change (2.0 preview):** removed
`HttpsTransportListener.IsWssTransportSupported` and
`ReplicatedGossipOptions.IsGossipStateDecodingSupported`. They only
returned `false` on netstandard2.1 and are now always `true`. The test
guards that used them were removed as well.

**Docs**
- NuGet READMEs, `README.md`, `DeveloperGuide.md`, `PubSub.md`, the
certificate/ECC/identity docs and the 2.0 migration guide (plus its
`.agents` skill mirror) are updated.
- The migration guide notes that 4.7.2 consumers must move to 4.8 and
netstandard2.1 consumers to net8.0+.

### Reviewer notes
- The `System.IO.FileSystem.AccessControl` `PackageVersion` pin is left
in `Directory.Packages.props`. It has no direct reference any more, but
transitive pinning is on, so it was kept to avoid shifting resolved
versions.
- The `#if` rewrite was done mechanically by a truth-table evaluator.
The diff in the `.cs` files is limited to directive lines, removed dead
branches, and stale-comment updates.

### Verified locally
- `dotnet build UA.slnx -p:CustomTestTarget=net48`: 0 errors.
- `dotnet build UA.slnx -p:CustomTestTarget=net10.0`: 0 errors.
- Default multi-target builds (no `CustomTestTarget`) of Opc.Ua.Types,
Bindings.Https, PubSub.Mqtt and Redundancy.Server succeeded.
- net10.0 tests:
  - Opc.Ua.Redundancy.Server.Tests: 840/840 passed.
  - Opc.Ua.Types.Tests: 11363 passed, 1 skipped.
  - ModelDependencyScanner tests: 20/20 passed.
- The full test suite was **not** run locally.

## Related Issues

- None

## 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.
- [ ] 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-10-02 17:09:49 +02:00
romanett 2ed505547b Fix flaky CancelledRecoveryCanBeReplacedWithoutSuppressingLaterKeepAlive test (#4596) 
## Proposed changes


`ManagedSessionReconnectTests.CancelledRecoveryCanBeReplacedWithoutSuppressingLaterKeepAliveAsync(False)`
failed on #4594 (a workflow-YAML-only PR) in [run
36590762260](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36590762260),
`test-windows-net10.0 (5/30)`:

```
Assert.That(harness.Logs.Records.Any(
    record => record.EventId.Name == "ManagedSessionKeepAliveFailureSuppressedWhile"), Is.False)
  Expected: False
  But was:  True
```

### Root cause (test race, not a product bug)

The suppressed record doesn't come from the failed keep-alive the test
checks. It comes from the recovery that this failure starts:

1. The scripted keep-alive read returns `BadNoCommunication`.
`ManagedSession.OnInnerKeepAlive` does not suppress it and calls
`TriggerReconnect`, as expected.
2. The outer recovery reactivates through the channel manager. When
`Session.ReconnectCoreAsync` finishes, `StartKeepAliveTimerAsync`
restarts the keep-alive timer with due time 0, which sends a read right
away.
3. The test reset `KeepAliveStatus` to Good only in the continuation
after `FailedKeepAlive`. That continuation runs asynchronously, so the
new read can still return Bad.
4. `ChannelEntry` goes `TransitionTo(Ready)` → `SignalReady()` →
`CompleteParticipantRecoveryAsync`, and only releases
`m_reconnectCoalescer` afterwards. A read released by `SignalReady` that
fails inside that window sees `ChannelRecoveryInProgress == true`.
Suppressing it there is intended: the manager is still restoring behind
its provisional Ready gate.

The assertion scanned every log record, so it also caught this correctly
suppressed failure from the follow-up recovery.

**Proof of causality:** adding `Task.Delay(100)` after `SignalReady()`
in `ChannelEntry`, plus `Task.Delay(50)` before the test reset
`KeepAliveStatus`, made both test cases fail every time with the CI
assertion. The log dump showed `KeepAliveReadFailed` →
`ManagedSessionKeepAliveFailureSuppressedWhile` right after the second
recovery's `Reactivating → Ready`. Both delays have been removed.

### Fix

- Check the suppression decision synchronously in a `KeepAlive` handler.
`ManagedSession` raises that event right after it decides whether to
suppress. The assertion now covers only the failure under test.
- Reset `KeepAliveStatus` to Good in that same handler, before the
triggered recovery can send another read.

## Types of changes

- [x] Bugfix (non-breaking change which fixes an issue)

## Checklist

- [x] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [x] I ran tests locally with my changes, all passed.

Local verification (net10.0):
- With the product-side repro delay still in place, both cases pass with
the fix.
- `ManagedSessionReconnectTests` (17 tests): 30 sequential runs, all
green.
- `CancelledRecoveryCanBeReplacedWithoutSuppressingLaterKeepAlive*`: 64
runs across 8 parallel workers, all green.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-09-30 03:44:18 +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
Marc Schier c4e0721a29 Add a documentation learning path and fix issues found while verifying it (#4555) 
## Summary

### Documentation
- Stage the documentation index as a learning path: a first application,
independent client, server, and PubSub paths, then data and services,
companion models, production, advanced integration, and draft or
experimental capabilities.
- Add Getting started (repository samples or NuGet packages), OPC UA
concepts, Your first information model, and a production readiness
checklist.
- Add Native AOT application publishing, a first PubSub publisher and
subscriber exchange, and the UDP transport options.
- Add or expand contents navigation throughout `docs/`, and keep
contributor documentation separate.
- Correct outdated SDK examples and capability guidance, consolidate
repeated explanations into canonical guides, and repair local links and
heading structure.
- Rewrite What's New in 2.0 as a concise feature overview with short
linked details; keep migration instructions in the Migration Guide.
- Expand the feature catalogue with specification IDs, GDS, LDS, the OPC
10000-21 onboarding registrar, ISA-95, xRegistry, OpenUSD, and
sample-local models; distinguish draft status, explain crypto
pluggability, and bring performance guidance forward.
- Address review feedback on wording, active monitored items during
shadow reload, and the `Samples.md` filename and references.
- Regenerate the `opcua-v20-migration` plugin's bundled copy of the
migration guides so it matches the edited `docs/migrate/2.0.x` guides.
- Merge the latest `master`, preserving upstream resource-isolation
guidance within the task-based documentation index.
- Retain deferred Lingity findings where appropriate; this PR does not
claim that every document meets the Lingity score thresholds.

### Fixes found while verifying the guides
Each fix has regression tests that fail without it.
- **Client:** V2 subscriptions compared their lifetime with the
operation timeout instead of the session timeout, and logged a spurious
warning. `ISubscriptionEngineContext`, which is new in 2.0, gains a
`SessionTimeout` property; implementations must add it.
- **PubSub:** a subscriber now registers the `DataSetMetaData`
configured on its readers (OPC 10000-14 6.2.9.4), and the UADP decoder
decodes DataSetMessages that carry no ConfigurationVersion. Before, a
secured subscriber of the reference sample received no DataSets.
- **Source generator:** a ModelDesign method that declares its arguments
inline, without a method type, produced code that did not compile; it
now uses `MethodState`. The generated code of existing models is
unchanged.
- **MinimalClient sample:** waits for its first data-change notification
before it disconnects.

### Known issues, not addressed here
- UDP multicast: the publisher does not send on the interface that the
subscriber joins, and `MulticastLoopback` defaults to false. The PubSub
guide uses unicast for the exchange on one computer.
- The UDP metadata announcement is sent unsecured on secured writer
groups; a secured subscriber drops it with a warning.
- A server does not start in globalization-invariant mode, because
`ResourceManager` creates `CultureInfo("en-US")`. The Native AOT guide
cautions against the mode.

## Validation
- `git diff --check` and
`./.azurepipelines/validate-migration-plugin.ps1` (15 bundled migration
docs validated, plugin manifests synchronized).
- Local Markdown links, anchors, file-link casing, and contents lists
checked across all 87 documents under `docs/`. The index links every
top-level guide and every target of the previous index.
- Both Getting started routes, Your first information model, the PubSub
exchange, and the Native AOT publish of the Getting started client and
server were run as documented. The NuGet route and the tutorial use the
public `2.0.0-preview.5` packages.
- The changed libraries build for all target frameworks in Release
without warnings.
- Tests on .NET 10: Opc.Ua.SourceGeneration.Tests 199 passed;
Opc.Ua.SourceGeneration.Core.Tests 3,977 passed, 8 skipped;
Opc.Ua.Server.Tests `Fluent` category 621 passed; Opc.Ua.Client.Tests
subscription-engine tests 70 passed; Opc.Ua.PubSub.Tests 1,374 passed, 2
skipped; PubSub fuzz replay 145 passed. The new and affected tests also
pass on .NET Framework 4.8.
- Earlier merge validation: 174 resource-isolation tests passed on .NET
10.
- The full solution suite was not run locally.

Review and validation artifacts are maintained outside the repository.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-09-28 19:19:39 +00:00
Marc Schier f9892020a3 Add configurable server resource isolation (#4541) 
# Description

Add configurable server resource isolation on top of the merged #4522
foundation.

- Coordinate connection, handshake, retained-message,
session-establishment, queue, execution, request-cost and parked-request
limits through one server-scoped provider.
- Keep configured startup, continuity/reconnect, control and
trusted-owner reservations inside existing totals. Shared capacity
remains usable up to explicit owner ceilings.
- Use authoritative Session membership for reassembly continuity
capacity without granting unrelated resource or rate privileges.
Preserve explicit owner mappings and bounded tracking.
- Cache immutable classification evidence while retaining current
Session and classifier checks.
- Support weighted request ordering or FIFO independently of resource
accounting. Installed providers enforce decoded-request limits in both
modes.
- Preserve fail-fast reassembly, valid Abort handling, fixed deadlines,
active-work protection, reverse-connect handoff and bounded OpenAPI
processing.
- Support custom-handler parking through `IRequestParkingPolicy`,
`WithRequestParking(...)`, and `RequestLifetime.ParkSink`. Parking
releases execution capacity only, keeping retained-data and parked
charges until completion. Built-in Publish and the global opt-out remain
unchanged.
- Provide direct, fluent and DI configuration, public incoming-request
parking support, effective startup limit logs and user-facing
documentation.

Balanced remains the default. SharedOnly without an explicit provider
retains shared behavior for intentionally incompatible/unlimited
configurations. Reservations are finite and are not one full-message
allowance per Session.

## Related Issues

Builds on merged #4522. Fixes #4554.

## Review and CI corrections

All three previously missed findings in [review
5326734718](https://github.com/OPCFoundation/UA-.NETStandard/pull/4541#pullrequestreview-5326734718)
are addressed in `9e25e4ecc`:

1. A custom FIFO provider receives RequestQueue, RequestQueueBytes,
RequestExecution and ParkedRequest accounting. A blocked head cannot be
overtaken.
2. `IResourceIsolationReassemblyProvider` replaces the sessionless
threshold independently of `UseFairScheduling`. The aggregate byte
ceiling remains enforced.
3. Runtime `ReservedReassemblyBytes` includes all configured protected
stage floors, including ControlReserved.

The review's two already-resolved changes remain intact.

The CI collection race was fixed at its source in `e1a994a4b`:
`ArrayOf.ToArrayOf` treats a separately read collection count as an
initial capacity hint and grows/trims for the actual enumeration.
Deterministic tests cover growth, shrinkage and stale-zero counts.

The subsequent recovery-test race was fixed in `05753ce77`: fake time
advances only after a gated keepalive response has been processed and
reset its timer, not merely after request arrival. Recovery-order and
timeout assertions remain intact, with additional response-state
assertions. Instrumentation reproduced the original failure twice; both
fixed variants passed 400 executions per framework.

CodeQL alert 388 was verified and dismissed as a false positive. Startup
logs contain aggregate numeric capacities, not credentials or caller
identities.

## Validation

| Scope | Result |
| --- | --- |
| Latest automated checks at `05753ce7775a58801b4ba9d8f6d7781b709d0b89`
| 167 successful, one package-publication check skipped, no failures |
| Remaining external prerequisite | `license/cla` pending: Contributor
License Agreement is not signed yet |
| FIFO/weighted queues, custom parking and server-channel selection |
175 passed on each net10.0/net48 |
| Isolation, configuration and parking-hosting selection | 259 passed
net10.0; 257 passed net48 |
| ArrayOf conversion tests | 73 passed on each net10.0/net48 |
| Recovery-audit and keepalive fixtures | 40 passed on each
net10.0/net48 |
| NativeAOT smoke from preceding updates | Published and executed
successfully |

No tests, assertions, coverage thresholds, or required checks were
weakened. The remaining CLA prerequisite must be completed through [CLA
Assistant](https://cla-assistant.io/OPCFoundation/UA-.NETStandard?pullRequest=4541).

This is not CTT certification or an unconditional network-DDoS,
whole-process-memory or fleet-wide guarantee.

## 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.
- [x] 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: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-09-27 21:11:52 +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 e0c56a7c00 Replay the fuzz crash corpus from the fuzz-corpus orphan branch (#4539) 
# Description

Since the move to GitHub Actions, CI replays only the checked-in fuzz
corpus. The full crash corpus used to be the Azure DevOps secure file
`FuzzingArtifacts.zip`, which `.azurepipelines/test.yml` overlaid onto
`Opc.Ua.Encoders.Fuzz.Tests`. The nightly `private-corpus` job that was
meant to replace it depended on an Azure storage account, federated
credentials and a protected environment. None of these were ever
provisioned, so the job never ran.

**The corpus now lives on the orphan branch
[`fuzz-corpus`](https://github.com/OPCFoundation/UA-.NETStandard/tree/fuzz-corpus)**
(commit `843ba944a7b144a7ae6429151846d9e5ab28d29f`, already pushed):
- It contains 22,267 `crash-*`, 25 `slow-unit-*` and 1 `timeout-*` input
under `Opc.Ua.Encoders.Fuzz.Tests/Assets/`.
- The files are byte-identical to the secure file (SHA-256
`85198e46…fe1d61b`), recovered from the `drop` artifact of Azure build
18397.
- The branch shares no history with `master`, so clones of `master`
don't get 22k extra files in their checkout.

**Why the corpus is no longer treated as private:**
- The inputs date from April–June 2024.
- The `drop` artifacts could be downloaded anonymously.
- Every master build printed up to about 7k base64 reproducers into
anonymously readable Azure logs.
- Current `master` replays every input without a test failure.

New, unfixed crashes still go through
[SECURITY.md](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/SECURITY.md)
and are added to the branch only after the fix ships.

## Changes

- **`nightly.yml`:** `private-corpus` is replaced by `crash-corpus`. The
new job:
- checks out the commit pinned in `FUZZ_CORPUS_COMMIT` (no secrets,
environment or OIDC);
- verifies the SHA and copies each top-level folder over
`fuzzing/<folder>`;
- runs `Opc.Ua.Encoders.Fuzz.Tests` on the 12 non-macOS profiles through
the shared `run-dotnet-tests` action and uploads
`crash-corpus-results-*`.

The `run_private_corpus` input and the INCOMPLETE label are removed. The
job runs on every scheduled and manual run, and the nightly summary
fails if it was skipped.
- **`fuzzing/CrashCorpus.md` (new):** the Azure history, branch layout,
how CI replays the corpus, local replay, and how to add inputs and move
the pin. `Fuzzing.md` links to it.
- **`docs/DeveloperGuide.md`:** the private-corpus section and its
secrets table are replaced, and the migration to-do list is updated.
- **`nightly.yml` AOT job (fix for a failure found by the first nightly
run):** the Linux and macOS legs looked up the published binary by
`BaseName`/`Extension`. A NativeAOT executable has no extension on those
platforms, so PowerShell splits `Opc.Ua.Aot.Tests` into `Opc.Ua.Aot` +
`.Tests`, and every leg reported *No NativeAOT executable was produced*
even though publish succeeded ([run
36017168717](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36017168717),
`aot-ubuntu-latest`). The lookup now matches the exact file name. A
failure now reports the publish errors, a missing publish directory, or
which files were produced instead.
- **`Opc.Ua.Redundancy.Samples.Tests` / `Opc.Ua.WotCon.Samples.Tests`
(fix for failures found by the first nightly run):** the test runner's
framework guard rejected both projects on the nightly net8.0, net9.0,
net472 and netstandard2.1 profiles. Redundancy built a net48 placeholder
for every target except net10.0, and WotCon was pinned to net10.0. Both
now build the requested target on net8.0/9.0/10.0 and use
`RestrictForLegacyTfm` to become explicit not-applicable shells on
net48, net472, netstandard2.0 and netstandard2.1. The redundancy tests
no longer hard-code `bin/<cfg>/net10.0` when locating the sample
binaries, and `SampleAppProcess` falls back to an `object` lock below
net9.0. The net48 placeholder remains for Azure's plain `dotnet test -f
net48` leg. Checked locally by building Redundancy for
`CustomTestTarget=net9.0` and WotCon for `netstandard2.1`.
- **`DefaultServerRedundancyHandler` (fix for a failure found by the
first nightly run):** `ResolveServerAsync` gave the resolver a token
linked to the caller's, but waited on the caller's token directly. On
cancellation the wait could finish, and its `using` could dispose the
linked source, before the linked source's own callback had run. That
unhooks it, so the in-flight peer lookup never saw the cancellation
(`PeerDiscoveryPropagatesCallerCancellationAsync`, netstandard2.0 build
on net48). The wait now uses the linked token. A timeout is still logged
as a discovery failure, as before. A companion test fix makes
`PeerTimeoutPreservesOtherEndpointsAndCoalescesPendingDiscoveryAsync(False)`
deterministic. The timed-out lookup drains on the thread pool, so the
test's retry could join it and then have its own time limit expire
before it started a fresh lookup. The test now waits for the second
resolver call before advancing the fake clock.
- **`Session` keep-alive logging (found while investigating the net48
nightly failures):** `OnKeepAliveError` and the keep-alive read failure
log used `Endpoint?.EndpointUrl`. `ClientBase.Endpoint` dereferences the
transport channel, so once the channel was detached it threw a
`NullReferenceException` on every keep-alive tick, and
`BadNoCommunication` never reached the `KeepAlive` subscribers. Both now
log the configured endpoint's URL. Three new `SessionKeepAliveTests`
cover a detached channel.
- **`ClientTestFramework` shared session (fix for 67
`Opc.Ua.Sessions.Tests` failures on the net48 nightly):** the
fixture-wide shared session used the 10 s default session timeout. A
test-host stall of about 11–18 s while other fixtures restarted their
servers let the server expire it, and every later test in the
`ClientTest("https")` and `ClientTest("opc.https")` fixtures failed with
`BadSessionIdInvalid`. The shared session now requests
`SharedSessionTimeout` = 120 s; per-test sessions keep 10 s.
- **`run-dotnet-tests.ps1`:** `-QuietOutput` help text only; it no
longer names the removed job.

The Azure secure file stays in place, because `master378` and the 1.x
release pipelines keep their own `test.yml` and still use it.

## Verification

- `nightly.yml` parses as valid YAML, and no job, output or summary
field still refers to the removed job.
- I ran the job's overlay script against a clone of `fuzz-corpus` at the
pinned commit. It overlaid all 22,293 files, left the tracked
`Assets/Repo` untouched, removed the checkout folder and wrote the
summary line.
- **Not yet run:** the workflow itself. It is
`schedule`/`workflow_dispatch` only, so it needs a manual `nightly.yml`
run on this branch. On master the corpus has so far only been replayed
on Windows (net48/net10.0 in Azure), so the Linux legs are new ground.

## Follow-ups (admin)

- Add a ruleset for `fuzz-corpus`: require pull requests and block force
pushes and deletion, so the pinned commit stays reachable.
- The `fuzz-private-corpus` environment, if it was ever created, can be
deleted.

## Related Issues

- No issue exists yet; this is part of the Azure DevOps → GitHub Actions
migration (#4513, #4524).

## 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.
- [ ] 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:00 +02:00
Marc Schier 57c0a32ed4 Fix incomplete-message buffering and channel cleanup (#4522) 
# Description

Fix incomplete-message buffering, channel cleanup, session resource
accounting, and enforcement of existing transport admission settings.

- Share a targeted `ChunkReassemblyBudget` across server listeners. The
reference default is 64 MiB, with a 32 MiB global occupancy threshold
for sessionless intermediate chunks. This is priority headroom for
activated sessions, not a guaranteed bootstrap partition. Single/final
chunks and unrelated response/client buffers remain outside this budget.
- Retain precise incoming/final payload quotas, valid Abort handling
without channel teardown, terminal late-final rejection, and
exactly-once buffer/reservation cleanup.
- Use one assembly timer per quota/clock scope, asynchronous cleanup, a
fixed deadline, a 30-second fallback for non-positive lifetimes, and
best-effort ERR `BadTimeout` before closure.
- Apply negotiated TCP/WebSocket/Pipe receive limits and size pipe
rentals to actual chunks.
- Add the optional `ISessionBindingProvider` and immutable
`SessionBindingContext` seam. Managed membership counts distinct
committed sessions, not responses, and handles reactivation, transfer,
closure, expiry, restoration and shutdown. Read-only snapshots require
token/channel/security validation and never replace service
authorization.
- Wire default and explicitly injected binding providers through raw
TCP, Kestrel and UACP WebSockets; preserve existing callback/manager
interfaces and direct construction.
- Enforce existing `ConnectionRateLimiter` and channel capacity settings
on Kestrel TCP and UACP WebSocket admission, with physical-transport
leases surviving reverse handoff and releasing on cancellation, close,
failed attachment and stop. HTTP/JSON requests are not treated as UASC
channels.
- Add regression coverage and documentation. Stabilize the CI
cancellation test by waiting for the resolver to enter before canceling.

This is the foundation only. New Balanced/FairShare/TrustedReservations
policies, tenant isolation and guaranteed bootstrap reserves belong in
the separate stacked follow-up, not this PR.

## Related Issues

Fixes #4521.

## Validation

| Scope | Result |
| --- | --- |
| Core transport, budget, binding-context and DI tests, net10.0 | 717
passed |
| Core transport, budget, binding-context and DI tests, net48 | 664
passed, 6 platform-specific skips |
| Session bindings, transfer/security/expiry/restore and hosted DI,
net10.0 | 69 passed |
| Same focused Server suite, net48 | 69 passed |
| ServerRedundancyHandlerTests, Release/net48 | 39 passed |
| ConsoleReferenceServer net10.0 | Build succeeded, zero warnings/errors
|
| Server and HTTPS binding netstandard2.1 | Both builds succeeded, zero
warnings/errors |

The new admission fixtures also passed five repeated runs after
correcting asynchronous connection-context disposal. Full-solution
validation, coverage comparison and CI for this update remain pending.

## 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.
 2026-09-24 20:12:55 +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
Cristi Pogacean 8889ffee0e Fix connector migration type handling and transfer recovery (#4496) 
Preserve null enumeration arrays while restoring enum metadata,
reconstruct concrete structure collections, and schedule transferred
notifications for immediate republish.

Add regression coverage for issue #4494.

# Description

_Describe the changes here to communicate to the maintainers why they
should accept this pull request. By default - this will become the
Commit message after merging and thus define history._

## Related Issues

_Reference all GitHub issues this PR addresses. If there is no issue
yet, open one and link it here._

_If this is a relatively large or complex change, a design must have
been discussed in the related tracking issue and signed off (which
becomes the Architectural Decision Record (ADR))._

- Fixes #github-issue-number, ...

## 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.
- [ ] I have added all necessary documentation.
- [ ] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.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-18 04:15:34 +00:00
Marc Schier fa8d45f2cf Fix server/core validation, lifecycle and protocol regressions (#4477) 
# Description

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

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

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

The approved acceptance cleanup is separate in the commit history:

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

## Latest review follow-up

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

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

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

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

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


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

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

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

The implementation questions are documented at their source:

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

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


### Detailed disposition of review 5191795284

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

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

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

## Earlier review follow-up

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

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

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

### Review-fix validation

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

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

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

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


## Latest upstream merge

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

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

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

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

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

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

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

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

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

Three conflict resolutions preserve both sides:

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

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

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

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

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

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

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

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

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

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

Local merge verification:

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

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

## Previous upstream merges

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

The overlapping fixes are reconciled rather than replacing either
branch:

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

## Validation

Local validation of the conflict-resolution merge:

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

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

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

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

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

## Related Issues

- Fixes #4476

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [x] I have addressed **all** PR feedback received.
 2026-09-16 04:42:09 +00:00
romanett ea93c86da2 Fix the Opc.Ua.Client defects reported by the client stack audit (#4471) 
# Description

Fixes the 78 `Opc.Ua.Client` findings of a read-only bug audit of
`Opc.Ua.Client` / `Opc.Ua.Types` / `Opc.Ua.SourceGeneration` taken at
a05a36055, plus the regressions a subsequent review of those fixes
turned up. Every behavioural change was checked against the OPC UA
specification, and the citations are in the code comments.

## Session, reconnect and failover

- The reconnect `finally` re-acquired `m_reconnectLock` with the
caller's token, so a cancelled reconnect left the session stuck in
`Reconnecting` forever and every later reconnect failed. It now uses
`CancellationToken.None`.
- An in-place session recreate tore down subscriptions it should not
have: `transferred` is all-or-nothing, so a single failed transfer reset
and recreated the ones the server had just taken over, orphaning them on
the new session. `TransferSubscriptions` now reports the
per-subscription outcome and only the subscriptions the server did not
take over are recreated. A token-reuse failover keeps the same server
session, so nothing is transferred or reset there at all.
- In-place recreate only tears down the channel it created itself; a
caller-supplied channel or a channel-manager lease is left to its owner.
- `Snapshot(out SessionState)` went through the `SessionOptions` record
copy constructor, which drops every member declared on `SessionState`,
so the paired `Snapshot`/`Restore` API could not round-trip.
- The user-token policy override restored the policy URI on failure but
had already disposed the ECC ephemeral key the previous identity needs,
so the identity the rollback claims to preserve could no longer be
reactivated. The key is now parked and restored.
- `SetTriggering` batching sliced the remove list past its end when a
call had more adds than removes (Part 5 §6.3.11 bounds the *sum* of both
arrays, so an individual list may be shorter than the limit).
- The certificate-less `None` attempt dropped the request header, and
with it the ECDH ephemeral key that travels in `additionalHeader` (Part
4 §7.1/§7.15).
- `Session.Equals` compared a `ConfiguredEndpoint` against an
`EndpointDescription`, so a session did not equal a session on the same
endpoint.

## Managed session, pool and connection state machine

- `ManagedSession.Dispose()` blocked on the connection state machine's
worker, which raises `ConnectionStateChanged` synchronously — disposing
from that handler deadlocked permanently. The machine now recognises its
own flow and completes the teardown as the worker's continuation.
- `ManagedSessionFactory` dropped `updateBeforeConnect`, `connection`
and `reverseConnectManager` on all four `CreateAsync` overloads.
- The session pool ran its shared connect on no token at all, so once
the only caller gave up nothing could abort it and
`RemoveAsync`/`DisposeAsync` waited out the whole reconnect budget. Each
entry now owns the token that aborts its connect, and removal closes
*and* disposes the session.
- An explicit `engineFactory` was silently ignored for
`DefaultSessionFactory` subclasses, which quietly downgraded the session
to the classic subscription engine.

## Subscriptions

- Fixed a lock-order inversion between `m_subscriptionLock` and the
partition locks in `CompleteAsync` and `DisposeAsync`.
- `OwnsSubscriptionId` stopped at the first registered subscription
carrying the id — but in the very case it guards, the asker still holds
its own retired id, so the answer depended on `HashSet` enumeration
order and the delete could still tear down a sibling that had been
handed the recycled id.
- `MessageProcessor`'s republish gap walk was unbounded; it is now
bounded by the server's retransmission queue (Part 4 §5.14.5).
- A publish acknowledgement is sent even when a notification handler
throws — the ack means "the server may delete the message" (Part 4
§5.14.5.2), not "I processed it".
- Loaded monitored items did not raise the process-wide client-handle
counter, so a fresh process minted handles that collided with the loaded
ones; a transfer-restored item was also stamped `BadOperationAbandoned`
and left at the default monitoring mode.
- `AutoSetQueueSize` was not idempotent, so the queue size ratcheted up
on every subscription state change.
- The snapshot dropped the partition and recovery options entirely, and
clamped the sampling interval and the secondary-partition idle timeout
to zero — turning the Part 4 §7.21 "use the publishing interval"
sentinel and "never delete an idle partition" into something else.
- The state stream is positional OPC UA binary and the reader cannot
detect an absent field, so appending the partition options would have
mis-decoded every previously saved file. The stream now carries a format
version and the first format is decoded through a frozen copy of its
shape.

## Browse, node cache and the remaining clients

- Managed browse wrote a retry pass's status into whatever node happened
to sit at that offset in the caller's list, and retried
continuation-point errors without any bound. It now keeps the caller's
slot and gives up on passes that make no progress — a fixed pass cap
would cut short a large browse against a conforming server with a small
continuation-point quota.
- Continuation points are released when a consumer stops enumerating
early, as Part 4 §5.9.3.2 requires.
- The node cache reordered its batched results relative to the inputs,
which mispairs them for every consumer that matches positionally; its
supertype walks could also spin forever on a server whose `HasSubtype`
chain loops back on itself.
- A browse error was cached as "this node has no references"; it is now
surfaced to the direct caller, while the bulk and hierarchy walks treat
the node as having no references for that one operation without caching
it, so a single un-browsable node cannot fail a whole batch.
- `WebApiClient` mutated a caller-supplied (typically
`IHttpClientFactory`-pooled) `HttpClient`, which throws once that client
has sent its first request; those instances are configured per request
instead.
- `PathCache` was not thread-safe.

## Review follow-ups

The last two commits are the result of reviewing the audit fixes
themselves. Worth calling out: the `Task.WaitAsync(CancellationToken)`
polyfill in `Opc.Ua.Types` never checked `IsCompleted`, so an
already-finished task plus an already-cancelled token reported
cancellation on every pre-net8.0 target where the BCL overload returns
the result. It is now aligned with the BCL and pinned by a test.

## Related Issues

- No tracking issue; this PR is the result of a read-only audit of the
client stack.

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

### Verification

35 regression tests were added under
`tests/Opc.Ua.Client.Tests/AuditRegressions/` (category
`AuditRegression`), one per fixed defect where the defect is observable
at unit level; most were mutation-checked by reintroducing the bug and
confirming the test fails.

Locally on **net10.0**: `Opc.Ua.Client.Tests` 2252/2252,
`Opc.Ua.Client.ComplexTypes.Tests` 2773/2773,
`Opc.Ua.Bindings.Https.WebApi.Tests` 346/346, and all 23 library and
test projects that reference `Opc.Ua.Client` build clean.
`Opc.Ua.Client` also builds clean for **netstandard2.1**, the target
that actually compiles the `WaitAsync` polyfill. The full **UA.slnx**
solution and the .NET Framework targets have not been run locally — left
to CI.

One pre-existing failure in `Opc.Ua.Types.Tests` is unrelated to this
branch:
`WotSpecFixtureManifestTests.EveryEmbeddedExampleMatchesItsRecordedHash`
(embedded WoT fixture hashes) fails identically with these changes
reverted.

### Notes for reviewers

- Two review findings were deliberately left unfixed and are documented
in the code: `ManagedSession.CreateAsync` has to write the engine choice
onto a caller-supplied `DefaultSessionFactory` subclass (the only
alternative is changing the `ISessionFactory.Create` contract), and the
connection state machine's "am I on my own worker" flag is an
`AsyncLocal`, so it also flows into work a `StateChanged` handler
spawns. The previous behaviour in that second case was a hard deadlock.
- `Session.Dispose()`'s single `GetAwaiter().GetResult()` bridge to
`CloseAsync` is left as is: `Dispose(bool)` must be synchronous and
`DisposeAsync` already exists.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
 2026-09-14 08:22:28 +02:00
Marc Schier 7411654c8d Complete OPC UA Part 11 Historical Access (#4390) 
# Description

Completes the OPC UA Part 11 Historical Access work requested by #4387
and
the historian framework integration requested by #4400.

The conformance/evidence catalog covers all 37 released UACore 1.05
Historical Access profiles: 15 Server facets and 22 Client facets,
together
with the standard aggregate functions and their conformance units.
Server
facets are advertised only when the resolved providers expose their
required
interfaces and capabilities.

## Server and provider support

- Completes raw, modified, at-time, processed, annotation, structured,
and event history.
- Dispatches best-effort, bulk, and transactional updates with aligned
statuses, old values, diagnostics, and rollback state.
- Supports mixed history-update detail types and batched
structured/annotation operations.
- Uses composite identities and exclusive cursors for same-timestamp
structured, modified, and event values.
- Captures live data and events, installs historical configuration
objects, and emits typed audit payloads.
- Derives aggregate monitoring filters from per-node historian
capabilities and rejects unsupported processed aggregates before
provider dispatch.
- Supports tri-state `Historizing`: set, clear, or preserve
provider-owned state.

## Client support

- Exposes modified values together with `ModificationInfo`.
- Adds event-history reads and event insert/replace/update/delete
operations.
- Adds structured and annotation batching with strict response
validation.
- Shares continuation handling and release-on-abandon across read
families.
- Accepts inherited standard event fields rooted at event subtypes.

## Review hardening

- Registers DI-selected historians before node-manager reconciliation
and capability publication, while preserving provider ownership.
- Uses committed Raft read barriers for linearizable `TryGetAsync` and
`ScanAsync`.
- Keeps claimed continuation state immutable until a fresh successor is
saved, restoring the original after transient failures.
- Uses exact-incarnation compare-and-delete so delayed cleanup cannot
remove a restored same-ID continuation.
- Cleans up indeterminate initial and replacement saves, and propagates
shutdown cancellation through cleanup resolution.
- Preserves legacy continuation envelopes/cursors and distinct
annotation parent/property identities.
- Deletes every structured entry at a requested timestamp and uses the
complete modified-history ordering tuple.
- Shares native/fallback at-time and AnnotationCount calculations,
including reverse endpoints and cross-framework fractional intervals.
- Primes modified aggregate items before buffered live delivery and
deduplicates history/live overlap using provider-specific identities.
- Keeps committed Modify results successful while reporting priming
failures
  through the monitored item's notification queue.
- Keeps protected-notification state live-only for queues larger than
one.
Queue size one retains the newest notification without pinning errors or
transferring priority during resizing. Durable restore retains ordinary
definitions, last values/errors, and raw queues without recreating
priority.
- Isolates aggregate calculation, modification preparation, and
historical/live
handoff in `AggregationFilterHandler`, retaining existing
provider/factory
wiring and supported past-start behavior. The larger-queue protection
policy
  is unchanged pending the open review discussion.
- Uses one `IMonitoringFilter` owner for protocol filters or the
aggregation
handler, eliminating parallel effective-filter storage and retaining the
current definition during preparation. The original client filter stays
distinct.
- Integrates current master, retaining custom monitored-item creation
decisions and per-node history callbacks.

## Redundant-server alignment and samples

- Adds a protected shared historian with immutable segments, manifest
CAS publication, writer fencing, and restart recovery.
- Persists portable continuation state before exposing it to clients and
transfers ownership during mirrored-session takeover.
- Rejects eventual-consistency and active/active multi-writer historian
configurations rather than silently degrading.
- ReferenceServer and ConsoleReferenceClient demonstrate discovery,
paging, raw/modified/at-time/processed reads, annotation/structured
CRUD, event capture, and event CRUD.
- RedundantServer and RedundantClient demonstrate active/passive
history, continuation recovery after replica termination, and
post-promotion writes.
- Updates Historical Access, High Availability, profile, migration, and
sample documentation.

## Compatibility

- New 2.0 collection boundaries use `ArrayOf<T>` and public byte
payloads use `ByteString`.
- Removes the legacy public virtual
`SamplingGroupManager.CreateMonitoredItem` /
`ModifyMonitoredItem` entry points and protected creation factory, as
agreed
  in review. Migration guidance points to the monitored-item manager and
  construction-decision APIs.
- Retains established persisted formats and current write layouts.
Retired interim
notification-state formats (sample v2 / shared v4) are rejected, as
agreed in review.
- Keeps the historian integration compatible with .NET Framework and
NativeAOT.

## Validation

### Latest CI source repair

`73c619435` forwards the existing cancellation token at both
predefined-node
creation calls in the PR-added History event fixture. The merged
lifecycle
overload exposed these two CA2016 errors, which blocked History jobs,
the
solution build matrix, and CodeQL on GitHub and Azure. The exact
compiler
failure was reproduced locally; the repaired fixture passes 4/4
integration
cases on net10.0 and 4/4 on net48 with clean scoped diagnostics. No
assertions,
tests, coverage thresholds, or CI configuration were weakened.

The repaired History jobs on Linux/macOS and all four Azure solution
builds
now pass. The Windows all-TFM GitHub job instead lost its hosted
runner's
connection; no build log was recoverable, and GitHub recorded a
runner-loss
annotation rather than a compiler/test diagnostic. One unchanged retry
was
requested. Remaining checks are monitored with
`gh pr checks --watch --fail-fast --interval 30`; CI is not yet reported
as green.

### Latest base integration

`bd52e5fea` merges upstream `master` at `da7412c42`. The lifecycle
conflict
retains upstream's shared cancellation-aware registration helper and the
historian behavior. The hosted-historian fixture forwards its existing
token
to the new lifecycle overload. GDS project renames and all three AOT
test
hosts are retained, with their project references resolving after the
merge.

| Integrated scope | net10.0 | net48 |
| --- | --- | --- |
| Server lifecycle, fluent authoring/import, historian, monitored-item
and hosting regressions | 732 passed | 732 passed |
| NodeState lifecycle regressions | 216 passed | 216 passed |
| Automatically merged Raft client fixtures | 28 passed | 28 passed |

No selected tests failed or were skipped. The manually resolved files
pass
scoped formatting/analyzer checks. One CA1861 warning remains in the
incoming
`NodeSetImportIntegrationTests.cs`; that file is byte-identical to
upstream
master and was not rewritten or suppressed during integration.

### Latest feedback follow-up

`102954eaa` replaces split filter references with the shared
`IMonitoringFilter`
contract implemented by protocol filters and `AggregationFilterHandler`.
Targeted server regressions pass **103/103 on net10.0 and 103/103 on
net48**;
scoped formatting/analyzers are clean, and the updated XML-documentation
inventory has zero gaps across 188 non-generated PR C# files.

`93c5b5db8` renames `AggregationFilterHandler`, makes it the owner of
the
server-revised aggregate filter while preserving the original client
filter,
and removes the retired interim notification-state readers. Server
regressions
pass **101/101** and shared-store regressions **64/64**, on both net10.0
and
net48, with no failures or skips. Runtime-file formatting/analyzers are
clean.

`5ccf45630` completes the public/internal XML-documentation sweep. The
Roslyn
inventory finds zero undocumented eligible declarations across all 186
non-generated PR C# files; the 110-file documentation slice changes only
XML
comments and spacing. `f17765475` applies the three requested Markdown
edits.

### Earlier feedback follow-ups

`a53950737` removes single-slot notification pinning and extracts
aggregate
implementation from MonitoredItem. Targeted sampling, aggregate
modification,
history/live handoff, queue/lifecycle, restore, and serialization cases
pass
**101/101 on net10.0 and 101/101 on net48**, without failures or skips.
Scoped formatting/analyzers are clean; the focused review found no
significant
issues. The final formatted monitored-item fixtures also pass 45/45 on
net48.

The preceding `b21555568` removes the legacy sampling API bridge and
durable
notification-priority state. Its targeted Server cases passed 97/97 and
shared
subscription-store cases passed 64/64 on each framework. The broader
evidence
below remains pinned to its original tested commit, not attributed to
these
follow-ups.

### Earlier integrated-implementation validation

Validated implementation commit: **`3efae597f`**. The following commit,
`f6dbefb1c`, only adds two explanatory comment lines for the final
standards
review; it changes no executable code.

| Scope | net10.0 | net48 |
| --- | --- | --- |
| Server | 5,187 passed; 56 skipped | Affected surface: 2,160 passed; 1
reproduced baseline failure; 56 skipped |
| Client | 2,202 passed; 2 skipped | 2,204 passed; 5 skipped |
| History integration | 519 passed; 21 skipped | 519 passed; 21 skipped
|
| Redundancy server | 641 passed | 641 passed |
| Redundancy client | 124 passed | 124 passed |
| Core shared-store regression suite | 23 passed | 23 passed |

All six affected samples passed non-incremental Release/net10.0 builds
with
zero warnings and errors: ConsoleReferenceClient,
ConsoleReferenceServer,
RedundantServer, RedundantClient, MinimalBoilerServer, and
PumpDeviceIntegrationServer.

The published self-contained win-x64 NativeAOT executable is 116,924,928
bytes and has no CLR header or managed runtime dependency. Direct
execution
passed **HistoryAotTests 3/3** and **RaftAotTests 2/2**.

Final standards and specification reviews found no confirmed outstanding
high/medium functional gaps or hard standards violations. The remaining
constructor-purpose clarification is addressed in `f6dbefb1c`.

### Reproduced baseline exceptions

The net48 hosted-startup timeout also reproduces in a pristine worktree
at
the previous PR head, `24cf1416d`, in

`RegisterPostStartRegistriesWiresHistorianAndAliasStoresOnPlainServerAsync`,
with the same subsequent certificate-leak assertion. The corresponding
renamed case fails before startup in the reviewed branch as well. This
is
reported separately from the net48 affected-surface results, not counted
as
a passing full Server suite. The net48 matrix selected the historian,
aggregate, monitored-item, fluent, and continuation surfaces and
excluded
the Hosting namespace.

`DurableDataValueQueueVerifyReferenceBatchingAsync` failed at item 1000
in
the net48 affected-surface run and passed its single isolated rerun. The
same
failure at the same assertion reproduces on pristine `849e16acc`; the
test,
sample queue, queue factory, and batch persistor are unchanged. The
original
failure remains in the table rather than being erased by the rerun.

### CI source repairs

- `4be7eb8f7` keeps diagnostics/configuration managers out of
distributed
address-space replication. Those managers inherit the source interface,
but their live server state and redundancy metadata must remain
replica-local.
  The ownership regression passes on net10.0 and net48, and all nine
  redundancy sample CI tests pass locally.
- `d17f467b3` completes the sample startup repair: core managers are
also
replica-local; the startup banner no longer advertises readiness before
initialization; the producer starts after distributed services; and
native
Raft startup no longer forces disruptive extra elections. The full
sample
suite passed 9/9, with initialized `RedundancySupport=Hot`, and focused
  Raft/address-space regressions passed 8/8 and 3/3.
- `81b0b72be` moves the unchanged distributed-historian NativeAOT
scenario
into a smaller, always-run companion executable to bound Apple's linker
input. All 137 AOT tests remain present; the moved test and helpers are
  unchanged. GitHub and Azure publish/run both native executables, with
separate result directories where they share a job. The companion passed
actual Windows NativeAOT execution. Linux and Intel macOS CI passed both
  executables, but this split alone did not fix ARM64 macOS linking.
The remaining historian graph is rooted by ReferenceServer startup, not
just the relocated test. No test exclusion or speculative linker setting
  was added to conceal it.
- `8a5842913` isolates the independent MCP test graph into an always-run
native host. The existing MCP source and assembly leak hooks are linked
  unchanged, and evaluated compiler inputs prove exactly one owner for
  every test source: 133 main + 3 MCP + 1 historian = 137 declarations.
All three hosts run on every AOT platform. The main Windows native
object
is 9.35% smaller; both changed publishes had zero diagnostics. MCP
passed
  3/3; the identical main executable passed 133/133 on one unfiltered
followup after two initial endpoint-discovery connection failures, which
  remain recorded in the local evidence.
- The previous Intel macOS job successfully published and executed its
native
tests, then failed on artifact-upload DNS (`ENOTFOUND`). No upload gate,
  shared environment setting, or cache configuration was disabled.

The first repair round also encountered the unrelated
`MetricsAreEmittedForChannelLifetimeAsync` timing failure on macOS Core.
Neither that test nor channel-manager code is changed by this PR, and
its
assertions remain intact. It passed unchanged on the final repair head.

On `d17f467b3`, the Linux and macOS redundancy sample jobs pass; the
remaining
failure was ARM64 Apple's `too many large addends` assertion. The
verified
upstream relocation-anchor fix (dotnet/runtime#124721, backport
dotnet/runtime#132171) is not present in the shipping ILCompiler
10.0.11.
An experimental multi-module probe failed and was rejected, not added to
CI.
On `8a5842913`,

[`aot-macos-latest`](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/34102158680/job/101678934057)
successfully publishes and executes all three native hosts on ARM64:
136 main runtime cases, 1 historian case, and 3 MCP cases, with no
skips.
The previous Apple linker blocker is resolved. Linux NativeAOT also
passes.
No assertion, test, shared environment setting, cache, or required gate
was
weakened.

The same CI run reports a separate macOS Sessions teardown failure: all
662 executed tests pass, but the global leak assertion detects one
undisposed certificate. The MCP partition changes no runtime, Sessions,
or
test-fixture code. A single unchanged diagnostic rerun is queued once
GitHub
permits rerunning that job; the failure is not suppressed or counted as
green.
Other CI checks are still pending.

## Related issues

- Fixes #4387
- Fixes #4400

## Checklist

- [ ] I have signed the CLA and read the contributing guide.
- [x] I have added regression tests and directly related documentation.
- [x] I have retained working reference and redundant-server sample
wiring.
- [x] Final validation and review evidence is complete, including the
baseline exceptions above.
- [ ] All required CI checks pass; the fail-fast rerun for `73c619435`
is pending.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9dbd9527-d818-47a0-984c-fbb1472d3d6b
 2026-09-08 11:32:23 +02:00
romanett ede0b68154 Fix reverse-connect certificate leak and stale-connection retry (#4386) 
Two related robustness fixes on the reverse-connect / UA-SC handshake
path, both surfaced by the Sessions suite.

## 1. Certificate handle leak in the asymmetric header parse

`UaSCBinaryChannel.ReadAsymmetricMessageHeader` parses the sender
certificate chain (`Utils.ParseCertificateChainBlob`, allocating fresh
`Certificate` handles) and then runs several validations that can throw
— invalid/missing sender thumbprint, no matching receiver certificate,
receiver thumbprint mismatch, missing receiver thumbprint. The chain is
returned through an `out` parameter, so a throw never reaches the
caller's `using (senderCertificateChain)` in
`ReadAsymmetricMessageAsync`, and the freshly parsed chain is abandoned.
For a self-signed leaf that is exactly the intermittent `created ==
disposed + 1` seen on the Release leak gate under coverage
instrumentation. This is the third sibling of the two
concurrent-reconnect ownership leaks closed in #4356.

**Fix:** wrap the post-allocation region in a `try/catch` that disposes
the chain before rethrowing (validation logic unchanged).

**Test:**
`ReadAsymmetricMessageHeaderDisposesSenderChainWhenReceiverThumbprintMismatches`
crafts a header with a valid sender certificate and a mismatched
receiver thumbprint; without the fix it reproduces `created=3,
disposed=2` and trips the assembly leak gate, with the fix it passes.

## 2. Stale reverse-connection retry

A reverse connection the `ReverseConnectManager` delivers can already
have been closed by the server before the secure-channel handshake runs:
a server rotates and times out the outbound connections it offers, so
the one matched here — especially after an `updateBeforeConnect`
discovery round consumed an earlier one — may be stale. That surfaced as
an intermittent `BadConnectionClosed` ("Remote side closed the
connection") failing the whole connect (e.g.
`ReverseConnect2Async(true,true,…)`).

**Fix:** wrap reverse-connect session creation in
`DefaultSessionFactory` in a bounded retry (3 attempts) that requests a
freshly delivered connection when an attempt fails with a
connection-drop status (`BadConnectionClosed` / `BadNotConnected` /
`BadSecureChannelClosed`). Non-transient failures and cancellation still
surface immediately; the bound prevents an unbounded loop against a
genuinely unreachable server.

## 3. Test-host crash: NULL channel map in the inactivity timer

`TcpTransportListener.DetectInactiveChannels` (a fire-and-forget
inactivity-timer callback) iterated `m_channels` without holding
`m_lock`, while `CloseAsync` disposes the timer and nulls `m_channels`
under the lock. Disposing an `ITimer` does not join an
already-dispatched callback, so a late callback dereferenced the
now-null map and threw an unhandled `NullReferenceException` on a
thread-pool thread, crashing the process. This intermittently aborted
the `test-macOS-latest-Client` host.

**Fix:** snapshot `m_channels` and return early when null (mirroring the
existing guarded read on the accept path). Deterministic regression test
invokes the callback on a listener whose channel map is null (its state
before `OpenAsync` / after `CloseAsync`) and asserts it does not throw.

## Verification

- `Opc.Ua.Core.Tests` Stack.Transport/Client/Server: 2178 passed, leak
gate green.
- Coverage-instrumented `Opc.Ua.Sessions.Tests` (the CI leg): 775
passed, 0 failed, leak gate green.
- Reverse-connect suite (`ReverseConnect*`): 52 passed with the retry.
- `Opc.Ua.Core` and `Opc.Ua.Client` build warning-free (net10.0, net48,
netstandard2.1, net472).

Note: the reverse-connect failure is a rare timing flake (0
reproductions in 88 local attempts), so fix #2 addresses the root cause
identified from the ADO log and is validated by construction plus
regression rather than by a captured local repro.

## Related

- Follows #4356 (concurrent channel reconnect certificate leaks).

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Marc <marcschier@hotmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-09-02 12:24:42 +02:00
Copilot 029e4ce7f3 Fix ManagedSession.CloseAsync blocking during reconnect attempts (#4374) 
- [x] `ConnectionStateMachine.RequestClose()` cancels the
connect/reconnect/failover attempt in flight (linked close token)
instead of only flagging the state
- [x] `WaitForClosedAsync` is released whenever the machine reaches
`Closed`, including through dispose
- [x] `ManagedSession.CloseAsync(timeout, closeChannel, ct)` honours
`timeout` (bounded wait, `BadTimeout` on expiry) and `closeChannel`, and
returns the real close status; `CloseAsync(ct)` now closes through the
same path
- [x] `Session.CloseAsync` publish drain waits only for the requests in
flight at close time, skips when no response can arrive, and no longer
issues `Cancel` round trips for already-completed requests
- [x] Unit tests: `ConnectionStateMachineTests` (close cancels reconnect
in flight, close during connect) and `ManagedSessionTests` (close
completes while a connect attempt is in flight)
- [x] `docs/Sessions.md` documents the close semantics
- [x] Validation: Opc.Ua.Client builds for all TFMs;
`Opc.Ua.Client.Tests` (2167 passed) and `Opc.Ua.Sessions.Tests` (775
passed) green on net10.0
- [ ] `Opc.Ua.Subscriptions.Tests` run was still in progress when time
ran out

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

- Fixes #4364

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: romanett <romanett98@gmail.com>
 2026-09-02 09:28:45 +00:00
romanett 052576115d Fix ManagedSession.CreateAsync returning an unconnected session when the initial connect fails (#4350) 
# Description

When the **initial** connect of a `ManagedSession` failed, `CreateAsync`
did not throw. It returned a session with no inner `Session` behind it,
so every subsequent member threw `BadNotConnected` and the real error —
`BadCertificateUntrusted` in the reported case — was swallowed.

**Mechanism.** A failed initial connect moves the state machine
`Connecting` → `Reconnecting`, which calls
`ManagedSession.HandleReconnectAsync`. That method skipped its entire
body when `m_session` was `null` — exactly the state after a failed
initial connect — and fell through to `return ServiceResult.Good`. The
state machine read that as a successful reconnect, transitioned to
`Connected`, released the `CreateAsync` waiter, and handed back a
session that was never connected. The logs show it plainly: a
"Reconnected" line immediately after "Reconnecting" with no service call
in between.

The terminal path was broken as well: once the reconnect policy **and**
the failover were exhausted the machine settled in `Disconnected` and
nothing ever set the connected event, so `WaitForConnectedAsync` — and
with it `CreateAsync` — waited forever rather than surfacing the
failure.

**Fix.**

- `ManagedSession.HandleReconnectAsync` reads the inner session once
under the service lock and, when there is none, runs a full connect
(`HandleConnectAsync`) instead of reporting success. The reconnect
policy now genuinely retries the initial connect.
- `ConnectionStateMachine` tracks the error of the last failed
connect/reconnect attempt and sets a new *settled* event when the
connect cycle ends — either by connecting or by giving up
(`Disconnected` after a failed failover, or `Closing`/`Closed`).
`WaitForConnectedAsync` waits on that and throws a
`ServiceResultException` carrying the last attempt's error when the
connection was abandoned. A failover failure only supplies that error if
no attempt recorded one, so the `BadNotSupported` returned by a failover
with no redundant server configured never masks the actual cause.

`CreateAsync` can no longer hand back a session that reports connected
with `m_session == null`.

## Behavior change worth reviewing

A permanently failing initial connect no longer returns instantly:
`CreateAsync` retries under the configured policy and throws only once
`MaxRetries` / `MaxTotalReconnectTime` and failover are exhausted
(default budget 5 minutes). Callers wanting fail-fast cap the policy or
pass a cancellation token — documented in `docs/Sessions.md`.

No status code is treated as non-retryable. Classifying e.g.
`BadCertificateUntrusted` as fatal would break the very scenario in the
issue, where the connect can start succeeding once an operator trusts
the certificate.

## Tests

-
`ConnectionStateMachineTests.WaitForConnectedAsyncThrowsOriginalErrorWhenConnectGivesUp`
— the original error survives the failover's `BadNotSupported`.
-
`ConnectionStateMachineTests.WaitForConnectedAsyncThrowsWhenClosedWhileConnecting`
— a close while connecting releases the waiter instead of hanging.
-
`ManagedSessionTests.CreateAsyncThrowsInitialConnectErrorInsteadOfReturningUnconnectedSession`
— asserts the original status propagates out of `CreateAsync` and that
the connect was attempted more than once.
-
`WssOpenApiIntegrationTests.ManagedSessionOverWssOpenApiBearerRejectedWhenNoBearerAuthRegistered`
relied on the old behavior (it asserted `Connected == false` on the
session returned by a failed connect). Rewritten to bound the retry
policy and assert that `ConnectAsync` throws; the fail-closed HTTP 401
property it guards is unchanged.

Suites run locally on net10.0, all green: `Opc.Ua.Client.Tests` (2155),
`Opc.Ua.Sessions.Tests` (775), `Opc.Ua.Gds.Tests` (1096),
`Opc.Ua.Client.ComplexTypes.Tests` (2773),
`Opc.Ua.Redundancy.Client.Tests` (122). No new build or analyzer
warnings.

## Related Issues

- Fixes #4347

## 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>
 2026-08-31 12:02:02 +02:00
Copilot ca9cfcc933 Inject ISecurityPolicyRegistry into Session and secure channel (#4314) 
- [x] `ChannelQuotas` + `UaSCUaBinaryChannel` carry an
`ISecurityPolicyRegistry` (falling back to `SecurityPolicies.Default`)
- [x] Transport channel/listener settings carry the registry and feed
every `ChannelQuotas` construction site
- [x] `ClientChannelManager` and `ServerBase` source the registry
- [x] `Session`, `DefaultSessionFactory`, `ChannelManagerSessionFactory`
and `ManagedSession` carry it
- [x] Identity token handler injection points made load-bearing
(`UserIdentity`, `AsTokenHandler`, default identity providers,
`IdentitySelectionContext`)
- [x] `SessionSecurityPolicyHelper`, `ClientIdentityProviderExtensions`,
`CoreClientUtils`, `OfflineSecureChannel`,
`OpcUaSecurityKeyServiceClient`
- [x] Client and server DI wiring resolve `ISecurityPolicyRegistry`
- [x] `IPubSubSecurityPolicyRegistry` registered in DI and threaded
through the PubSub application/SKS paths
- [x] Channel asymmetric crypto (padding, sign, verify, ECC
classification) resolves from the negotiated policy instead of a
hard-coded policy URI switch
- [x] Unit tests for the registry carriers plus an integration test that
negotiates a policy registered through `AddSecurityPolicy` over a real
loopback handshake
- [x] `docs/CryptoProvider.md` documents how the registry reaches the
channel and the session
- [x] Full solution build (`UA.slnx`, net10.0) and full
`Opc.Ua.Core.Tests` run (4351 passed) are green
- [ ] Broader integration suites (`Opc.Ua.Sessions.Tests`,
`Opc.Ua.Server.Tests`, `Opc.Ua.PubSub.Tests`) were still running when
time ran out

- [x] Review feedback addressed: preserved original CLR signatures,
routed factory channel creation through public registry-aware overloads,
failed closed for unsupported asymmetric encryption, and added a
provider contract for custom session factories
- [x] Focused review-feedback tests pass (28 tests across Client, Core,
and Core.Diagnostics)

- [x] Security review: hardened SKS custom-policy admission against weak
algorithms and incoherent RSA/ECC key-exchange metadata; final
specialist review found no remaining vulnerabilities

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

- Fixes #4250

---------

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 <mschier@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: romanett <romanett98@gmail.com>
 2026-08-27 05:57:17 +02:00
romanett d0d048dbb3 Fix spurious BadNoCommunication reconnect loop on sessions with steady traffic (#4318) 
# Description

A session with steady request traffic that then goes quiet for one
`KeepAliveInterval` was flagged `BadNoCommunication` and driven
`Connected -> Reconnecting` by the `ConnectionStateMachine`, even though
the connection was perfectly healthy.

Two parts of the keep-alive logic disagreed about what counts as proof
of liveness:

- `Session.RequestCompleted` treats **any** good service response as
proof the server is alive and postpones the keep-alive Read by a full
`KeepAliveInterval`.
- The freshness timestamp `KeepAliveStopped` is judged against
(`m_lastKeepAliveTimestamp`) was refreshed **only** by an actual
keep-alive Read response in `OnKeepAlive` (and at timer start).

So under steady traffic the keep-alive Read was perpetually deferred
(the stated intent) while its freshness timestamp went arbitrarily
stale. The moment traffic paused, the timer finally fired,
`OnSendKeepAliveAsync` evaluated `KeepAliveStopped` against the stale
timestamp and raised `OnKeepAliveError(BadNoCommunication)` **before
attempting the Read**. `ManagedSession.OnInnerKeepAlive` then routed
that straight to `StateMachine.TriggerReconnect()`, producing a
dead-regular Connected -> Reconnecting -> Connected loop on an
idle-but-healthy session.

## What changed

`src/Opc.Ua.Client/Session/Session.cs`:

- `RequestCompleted` now refreshes the liveness timestamp alongside the
timer reset, so the same response that defers the Read also counts as
proof for the `KeepAliveStopped` freshness check. Extracted into a small
`UpdateLastKeepAliveTime()` helper that the three existing refresh sites
(`StartKeepAliveTimerAsync`, both branches of `OnKeepAlive`) now share.
- The refresh is guarded by `if (!KeepAliveStopped)`, so a **latched**
keep-alive error is still only cleared by an actual keep-alive response
in `OnKeepAlive`. This matters: the keep-alive Read's own
`RequestCompleted` runs *before* `OnKeepAlive`, so an unconditional
refresh would have made `OnKeepAlive` take the `else` branch and
silently skip the recovery path (`StartPublishing`, defuncting stale
publish requests) after a `BadNoCommunication`.
- `m_lastKeepAliveTimestamp` is now read and written through
`Interlocked`, since it is written from arbitrary response threads
rather than only from the keep-alive worker.

`LastKeepAliveTime` / `LastKeepAliveTimestamp` therefore now mean "last
time the server proved it is alive" rather than "last keep-alive Read".
That is what both existing consumers already want:
`ClassicSubscriptionEngine` only logs it next to `KeepAliveStopped`, and
`SessionReconnectHandler` uses it to compute the remaining
session-timeout budget, which becomes more accurate.

## Test plan

New `tests/Opc.Ua.Client.Tests/Session/SessionKeepAliveTests.cs` (5
tests, driven by `FakeTimeProvider`, no server needed):

- `GoodResponsesKeepTheSessionAliveWhileTheKeepAliveReadIsDeferred` —
the reported repro: a good response every 2 s for 20 s with a 5 s
`KeepAliveInterval`. Confirmed this **fails on the unpatched code**
(trips at iteration 2, i.e. 6 s in) and passes with the fix. It also
asserts that once traffic stops, silence is still detected.
- `KeepAliveStoppedTripsWhenNothingIsHeardFromTheServer` — negative
control, the detector still works.
- `BadResponsesDoNotRefreshTheKeepAliveTimestamp` — only good responses
count.
- `LatchedKeepAliveErrorIsNotClearedByAGoodResponse` — unrelated traffic
cannot clear a latched keep-alive error.
- `RecoveryAfterBadNoCommunicationIsHandledByTheKeepAliveResponse` —
after `BadNoCommunication`, `OnKeepAlive` still enters the recovery
branch even though the keep-alive Read's own response passed through
`RequestCompleted` first.

Local runs: `Opc.Ua.Client.Tests` on net10.0 fully green (2133/2133).
`Opc.Ua.Sessions.Tests` (live in-proc reference server) was still
running when this PR was opened; I will follow up if it reports
anything.

## Related Issues

- Fixes #4317

## 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-26 19:19:34 +00:00
Copilot d525bc3f28 Bound reconnect cleanup and organize samples by domain (#4276) 
# Description

This PR fixes bounded ManagedSession reconnect cleanup and reorganizes
the sample applications into domain-focused folders.

## Reconnect cleanup

- Bounds reconnect integration cleanup with independent 30-second close
and dispose limits.
- Sends `CloseSession` through the awaited transport request path during
`Session.DisposeAsync`.
- Preserves explicit `Session.CloseAsync` behavior that waits for or
cancels outstanding publish requests.
- Covers the disposal request path with a regression test.

## Sample organization

- `samples/OpenUsd`: `GeneratorServer`, `SiteCompositionServer`
- `samples/Redundancy`: `RedundantClient`, `RedundantServer`,
`RedundantPubSub`
- `samples/Isa95`: `MinimalIsa95Server`
- `samples/MinimalApi`: `MinimalBoilerServer`, `MinimalCalcServer`,
`MinimalClient`
- `samples/DI`: `PumpDeviceIntegrationServer`
- `samples/Lds`: `ConsoleLdsServer`
- `samples/Reference`: `ConsoleReferenceClient`,
`ConsoleReferenceServer`
- `samples/PubSub`: `ConsoleReferencePubSubClient`

`UA.slnx` now exposes the same requested domain groups. Project
references, tests, runtime sample probes, CI workflows, Dockerfiles,
Compose files, helper scripts, and documentation were updated to the new
paths. `samples/Robotics/MinimalRobotServer` remains under `Robotics`.

## Validation

- `dotnet build UA.slnx`
- 3 Redundancy short-haul sample tests
- 70 DI sample tests
- 914 OpenUSD tests
- 1 WotCon model-materialization test
- 137 AOT tests
- `docker compose config` for all 6 moved Redundancy Compose files

## Related Issues

- Fixes #4273

## 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.
- [x] I built the complete `UA.slnx` solution locally.
- [ ] I ran all tests locally 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: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <mschier@microsoft.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Copilot-Session: a2c2838f-74ae-4997-a1d4-50574d9e0636
 2026-08-21 04:35:47 +00:00
Marc Schier bf9696e245 Fix the classic engine's publish pipeline: refill after keep-alive recovery, and bound every producer (#4274) 
# Description

`Session.OnKeepAlive` recovers a session whose keep alives had stopped by marking **every** outstanding publish request defunct — dropping `GoodPublishRequestCount` to zero — and then calling `StartPublishing` to refill the pipeline. Those written off requests never return.

The classic engine's publish reservation, added in #4232, could not observe that. It counted the requests the engine had sent and released a slot only when a request *completed*, so the write off left the reservations held forever: `TryReservePublishRequest` refused on the first iteration, `StartPublishing` issued nothing, and **the session never published again**. Recovery from a keep-alive outage is exactly the path that stops working.

That counter was a second, independent accounting of the same thing the session already tracks, and the two could not be reconciled. Both prior behaviours were wrong in one direction:

| `StartPublishing` | Outcome |
| --- | --- |
| Unbounded (as originally merged in #4232) | Overshoot — it runs once per subscription create and stacked a pipeline per call: 84 and 165 outstanding against a cap of 50 in CI |
| Bounded by the send-side counter (current `master`) | Permanent stall after a write off, as above |

## What changed

The reservation is now reconciled with the session's own accounting instead of duplicating it.

The engine tracks only the requests it has sent that the session has **not recorded yet**, and the limit is checked against `GoodPublishRequestCount + unrecorded`:

- `GoodPublishRequestCount` is authoritative and already honours the write off, so a written off pipeline refills immediately — the recovery valve behaviour is preserved as a property of the accounting rather than as an unbounded escape hatch.
- The unrecorded count covers the window before `AsyncRequestStarted` records a request, which is the lag that made the original check-then-act overshoot: concurrent callers all read the same stale value and each sent.
- The reservation is released when the request is **recorded**, not when it completes, so no engine-side counter can be stranded by a request that never returns.

Reads take the unrecorded count before the session count, so a request moving between the two is counted twice rather than missed — the error is always towards sending less.

### The third producer: the keep-alive nudge

Bounding the two automatic producers was not enough. CI kept failing with roughly twice the desired requests outstanding (90 and 100 against a cap of 50), which neither bounded producer could explain, so the remaining one was **instrumented rather than guessed at**: a local run logged **23** calls into `Subscription.HandleOnKeepAliveStopped`, which sends an *uncapped* publish whenever a subscription has seen no notification. One nudge per subscription adds a second full pipeline, which matches the observed numbers — and the server agrees the surplus is real: the failing CI run answered it with `BadTooManyPublishRequests` **59 times**.

That nudge now goes through the same reservation. Sending past the desired count cannot help, because the server already holds that many requests and rejects the rest. The limit is at least one, so an empty pipeline is always refillable, and because the reservation reconciles against the session's count, a pipeline whose requests were written off still refills — the case the nudge exists for.

**Behaviour change worth reviewing:** `ISession.BeginPublish` now returns `false` instead of sending when the pipeline is already at the desired count. The signature and the documented contract ("true if the request was sent") are unchanged, and `Subscription.HandleOnKeepAliveStopped` is its only in-tree production caller.

## Testing

`StartPublishingRefillsPipelineAfterRequestsAreWrittenOff` reproduces the stall deterministically: it fills the pipeline, writes the requests off the way `Session.OnKeepAlive` does, and asserts the refill. It issued **0 of 5** before this change.

`ConcurrentPublishReEvaluationDoesNotExceedDesiredRequestCount` (the overshoot guard from #4232) still passes, so both bounds hold at once.

`BeginPublishIsBoundedByTheDesiredRequestCount` covers the nudge from both sides: twelve nudges across three subscriptions issue three requests, and the nudge gets through again once the outstanding requests are written off.

Run locally on this branch:

| Suite | net10.0 | net48 |
| --- | --- | --- |
| `Opc.Ua.Client.Tests` | 2127 / 2127 | 2129 / 2131 (2 skipped) |
| `Opc.Ua.Subscriptions.Classic.Tests` | 32 / 32 | 32 / 32 |
| `ClassicSubscriptionEngineTests` | 25 / 25, five consecutive runs | — |
| `Opc.Ua.Client` build, all TFMs | 0 warnings, 0 errors | — |

`PublishRequestCountAsync` is the integration test that caught the overshoot in CI. Under deliberate CPU contention (6 burners on 8 cores) it was run repeatedly and **no run reproduced the overshoot**; the failures that remain under that load are setup and teardown timeouts on a starved machine (`BadRequestTimeout` in `CreateSubscriptionAsync`), not publish accounting.

## Related Issues

No tracked issue exists yet — this is a follow-up regression fix to #4232, found while investigating the `Subscriptions.Classic` CI failures on that PR. Happy to open one if maintainers would rather track it separately.

- Follow-up to #4232

## 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. _(the reservation itself is internal; the one behavioural change — `BeginPublish` returning `true` when the pipeline is already primed — is documented on the method's `<returns>`/`<remarks>` and called out under "The third producer" above)_
- [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. _(ran the affected suites on both TFMs as tabled above, not the full solution)_
- [x] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. _(the `PublishRequestCountAsync` overshoot is fixed at the source; the residual transport-timeout flake under heavy agent load is pre-existing and reproduces identically on the pre-merge parent — see the merge note below)_
- [x] I have addressed **all** PR feedback received.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3f5ab76a-c0ba-4f47-87ac-54d1f4182c6b
 2026-08-18 16:53:02 +00:00
Marc Schier aad8350bc1 Pluggable symmetric cryptography, an awaitable asymmetric path, registrable security policies, and PubSub crypto offboarding (#4206, #4207, #4208, #4210) (#4232) 
# Description

Closes the three crypto-offboarding follow-ups split out of #4192. Each
was recorded there as knowingly *not* done, with the reason and what
would justify revisiting it. Those conditions are met, and where one of
them turned out to be unachievable that is written down rather than
worked around.

The safety property throughout: **every seam is opt-in and null-checked,
and in the default software configuration the added code is observably
inert.** No interface dispatch on the per-message path, and the
asynchronous paths complete synchronously — so channel locking order,
buffer ownership and error sequencing are unchanged for a deployment
that configures nothing.

## #4207 — symmetric, key-derivation and RNG provider seams

`ISymmetricCryptoProvider`, `IKeyDerivationProvider` and
`ISecureRandomSource`, added on the terms #4207 set: only once a
concrete requirement arrived (a validated module that must perform
*every* operation, not just the asymmetric ones), behind a fast path,
with `SymmetricChannelCryptoBenchmarks` as the gate.

They are **facets discovered by type test**, not members of
`ICryptoProvider`, so providers written against the shipped interface
still compile. `PlatformCryptoProvider` implements all three, which
answers the "an interface nothing implements" objection recorded in the
issue.

`CryptoProviderFacets` returns `null` both when no registry is
configured *and* when resolution lands on the platform, because the
platform facets are the inline code. Resolution happens once, in
`CalculateSymmetricKeySizes`, and is held for the life of the channel.

`CryptoCompliance.GetUnservedOperationPurposes` closes the failure the
seam would otherwise introduce: a provider bound to `ChannelSymmetric`
without implementing the facet is silently replaced by the platform, so
a deployment could believe its validated module performed the
per-message cryptography while the platform did. Under `FipsOnly` that
now refuses to start.

## #4208 — the asymmetric path is awaitable

`RSA` and `ECDsa` are .NET's contracts and cannot be replaced without
making every ready-made hardware and cloud implementation unusable.
Instead an implementation may **also** declare `IAsyncRsaKey` /
`IAsyncEcdsaKey`, which the stack finds by type test.

Now asynchronous: the secure channel open and renew path, user identity
token signing and decryption, and session activation. A software key
declares neither facet, so those paths complete synchronously and
nothing about their ordering changes.

`ReadAsymmetricMessage` is refactored into shared helpers rather than
duplicated — adding an `await` was not worth two hundred cloned lines of
signature, padding and endpoint-selection logic. `AsymmetricMessage` /
`AsymmetricWriteResult` exist because an asynchronous method cannot have
`out` parameters.

The channel no longer serialises its state on a monitor. A monitor
cannot be held across an `await`, and a plain `SemaphoreSlim` was not an
option at the time: the code relied on re-entrancy in eight places
(`HandleIncomingMessage` holds the lock and calls `ForceChannelFault`,
which takes it again; `ForceReconnect` and `Shutdown` do the same).
`ChannelGate` replaces it at 32 sites and `DataLock` is `[Obsolete]`.
That reliance has since been removed as well — see [Removing the gate's
re-entrancy](#removing-the-gates-re-entrancy) below.

### Defects this surfaced, all fixed and regression-tested

**Detached work started inline.** Writes were started with `_ =
WriteBuffersAsync(...)`. An async method runs its prologue on the
caller's stack, so `LeaveInheritedContext` — which exists to stop
*detached* work inheriting the right to re-enter the gate — was
stripping the **caller's own** entitlement; when the send then completed
synchronously, the completion blocked on a gate that very thread was
holding. The client sent its OpenSecureChannel request and never
processed the response. Writes are now queued.

**A synchronous gate handle held across an `await`.** `Enter()` records
the acquiring thread so an inline completion callback can re-enter —
`ChannelAsyncOperation` invokes its callback both inline and detached.
That record is only sound while the holder is synchronously on that
thread; the token renewal path held it across an await, so a reused pool
thread was recognised as the holder.

**`SignAsync` rejected the policy that signs nothing.** It validated the
certificate before the algorithm, so `SecurityPolicies.None` — which
carries no certificate — threw where the synchronous `Sign` returns
`null`.

**`EnterAsync` lost the holder when contended** *(found in review)*. The
`AsyncLocal` recording ownership was written inside the awaiting method,
and .NET restores the caller's execution context when an async method
completes, so the write was discarded. The caller returned owning the
semaphore while `IsHeldByCurrentContext` reported `false` — and any
nested `Enter()` then blocked forever on a gate its own context held.
Reachable remotely: `HandleIncomingMessageAsync` → error path →
`ForceChannelFault`. The holder is now published from the caller's frame
before the wait, and only its depth is raised in the continuation.

**Queued writes lost their order** *(found in review)*. Sequence numbers
are assigned under the gate, but independently queued work items reach
the transport's FIFO send lock in arbitrary order, so a peer could
reject a chunk with `BadSequenceNumberInvalid`. Writes are now appended
to a chain: still off the caller's stack, but a write cannot start
before its predecessor finishes.

### Four more this surfaced in CI, on paths only the integration suites
reach

Queuing the writes to fix the deadlock above turned out to have moved
three implicit guarantees, and the gate had one more liveness bug. Each
was reproduced locally against a clean baseline before it was changed.

**A faulting channel's error message never reached the peer.** A
listener channel sends a UASC `Error` describing why it faulted and
closes the transport as its next statement. That worked only because the
write started on the caller's stack, so the bytes reached the socket
first; queued, the close discarded them and the client saw
`BadConnectionClosed` instead of the certificate status it was told to
expect. Nine `SecurityCertValidationTests` cases assert exactly that
status. The terminal error is written inline again, on a path that
touches the gate only to disclaim what it inherited and reports nothing
— a channel that is already faulted is owed nothing beyond its buffer.

**The certificate of a rejected sender leaked.** An asynchronous method
cannot have `out` parameters, so the asymmetric read returns a record
instead. That silently dropped an ownership guarantee: an `out`
parameter reaches the caller even when the method goes on to throw, so
the caller's `catch` disposed the sender certificate and reported it to
the audit. A return value does not — nine leaked certificates, one per
rejection case, which the leak detector fails the run on. The parsed
certificate is now handed to the caller before anything that can reject
the message, restoring both the disposal and the audit.

**The gate could strand its own permit.** It released only when the
holder's depth returned to zero, but that depth lives in an `AsyncLocal`
and is therefore shared with every context forked from the owner's — a
timer callback whose timer was created inside the region, for instance.
Such a context entering the gate raised the shared depth, so the owner's
own exit saw a non-zero depth and returned without releasing, and
nothing afterwards could: the nested exit is not the owner. Every later
entry on that channel blocked forever. `Opc.Ua.Features.Tests` hung
after its tests had finished, with a receive loop waiting on the gate
and the server unable to stop. The owner now releases when it leaves;
depth still gates re-entrancy, but liveness no longer depends on a count
another context can move.

**Write completion throttled the writer.** Reporting a completed write
sat inside the ordering chain, and the client channel implements that
callback by entering the gate — so every write queued behind a gate the
receive loop contends for on each incoming chunk. A session with fifty
subscriptions publishing continuously had its publish requests throttled
enough that the client kept topping them up past its own limit, which
`Subscriptions.Classic` asserts against. Completion is now reported off
the chain, which covers only what ordering actually requires.

### Still synchronous, and why

Certificate, certificate request and revocation list signing cannot be
made asynchronous at all: `X509SignatureGenerator.SignData` is invoked
by .NET's own builders. Service faults and the `Reconnect` override are
reached from synchronous call sites (`SendServiceFault` alone has eight)
and are left as they are behind scoped, commented suppressions. All
three are documented rather than left to be rediscovered.

## #4210 — PubSub

This one produced a finding rather than the API the issue asked for, and
the finding is the point:

> **With a standard Security Key Service the key necessarily exists in
process memory.** `GetSecurityKeys` (Part 14 §8.3.2) returns raw key
bytes over the wire, so the "key never leaves the device" property #4192
achieved for client and server **cannot** be achieved for PubSub through
the SKS pull profile. That is a property of the specification, not of
this stack.

A wrapped-key envelope would change what is on the wire and break
interoperability with third-party key services and publishers, so it was
rejected rather than built.

What is achievable is delivered: the per-message AES-CTR and HMAC route
through `ISymmetricCryptoProvider`, so a validated module performs them.
`IPubSubSecurityPolicy` is **not** modified — new members would break
every implementation of it; the provider is supplied through the policy
constructor. `PlatformSymmetricCryptoProvider` gained AES counter mode,
which the seam should have covered anyway since the algorithm enum names
it.

Key lifetime is also tightened: unpacking an SKS response copied key
material into intermediates that `ByteString.Create` then copied again,
leaving plain keys in the heap for the lifetime of the process. Those
are now zeroed.

### Two wiring gaps this depended on, both found in review

The registrations above were **inert**, so this section originally
claimed more than the code did:

- `CryptoProviderConfiguration` was applied nowhere outside tests, so
`AddCryptoProvider(configure)` never reached the registry in a real
host. The registry now applies every registered configuration when it is
first resolved — which also affects #4207's DI story.
- `PubSubSecurityWrapperResolver` selected
`PubSubAes256CtrPolicy.Instance`, the provider-less static singleton,
ignoring the provider-backed bundles registered for it. A `FipsOnly`
deployment therefore certified while the platform did the work. The
resolver now selects from the registered policies; the container and
`PubSubApplicationBuilder` both supply theirs.

A third, smaller one: `CryptoProviderBuilder.Use` published a
**policy-scoped** random source as the process-wide nonce source, so
`.For(purpose, policyUri).Use(hsm)` redirected nonce generation for
every other policy too. Only an unscoped binding does that now.

## #4206 — a provider can contribute a security policy

Added in review: the policy set was fixed at compile time, so a provider
could contribute key custody but not a *profile*. It is now table-driven
and open, and the blockers #4206 listed are gone.

```csharp
using IDisposable registration = SecurityPolicies.Register(
    new SecurityPolicyInfo("urn:vendor:SecurityPolicy#CustomProfile", "CustomProfile")
    {
        PlatformSupport = () => true,
        SupportedCertificateTypes = [ObjectTypeIds.RsaSha256ApplicationCertificateType],
        IsDefault = true
    });

services.AddOpcUa()
    .AddCryptoProvider(crypto => crypto
        .For(CryptoPurpose.ChannelSymmetric, "urn:vendor:SecurityPolicy#CustomProfile").Use(module))
    .AddSecurityPolicy(customPolicy);
```

- **The reflection is gone.** Four lookup tables were built by
reflecting over the fields of `SecurityPolicies` and
`SecurityPolicyInfo`. They are one explicit table now, which removes the
**last reflection in `Security/Constants`** — so this is a trimming and
Native AOT improvement as much as a feature.
- **`SecurityPolicyInfo` is constructible from another assembly** and
carries the metadata the helpers need, so `IsPlatformSupportedName`,
`GetDefaultUris`, `MapSecurityPolicyToCertificateTypes` and
`GetCurveFromCertificateTypeId` read that table instead of each being
hand-written. Adding a policy is one entry rather than five edits.
- **The dead `#if CURVE25519` conditional is deleted** from
`Security/Constants`.
- Registration is copy-on-write, so the read path stays lock-free.
Shadowing a built-in policy requires `replaceExisting: true` and is
reversible by disposing the registration.

`RegisterLightsUpCurvePoliciesFromOutsideCore` is the acceptance test
the issue asked for: it registers `ECC_curve25519` and `ECC_curve448`
from the test project and asserts they become discoverable through the
public API.

**Registering a policy makes it advertised and resolvable; it does not
supply the cryptography behind it.** For those two curve profiles the
in-tree key agreement is still behind a compile symbol no project
defines plus a BouncyCastle dependency, so a deployment lighting them up
provides the operations through a provider. That division is the point:
the policy set says *what* is offered, the provider says *who* performs
it.

## Removing the gate's re-entrancy

Folded in from #4242, which was merged into this branch.

`ChannelGate` was re-entrant, and that forced the design behind most of
the defects above. A holder that awaits releases its thread, so
ownership could not be tracked by thread identity — it had to live in an
`AsyncLocal<Holder?>`. A logical context is *inherited*, so anything
started while the gate was held inherited the right to re-enter and ran
inside the guarded region alongside whatever started it, unless it opted
out by hand via `LeaveInheritedContext()` at each of eight sites,
enforced by review alone. Five of the defects listed above trace to that
mechanism, including both CI hangs.

So the *requirement* was removed rather than the mechanism, and the
`AsyncLocal` then deleted itself. Every path that re-entered now calls a
lock-free `Core` method (`ForceChannelFaultCore`, `ShutdownCore`,
`ForceReconnectCore`, `CompleteConnectCore`), and three regions stopped
running foreign code under the lock: the `RequestReceived` dispatch is
deferred until the gate is released, `OnTransportError` no longer wraps
the `HandleSocketError` virtual, and the callback setters and
`EndpointDescription` publish a single reference with a volatile write
instead of locking — the last is load-bearing, because
`AsyncResultBase.OperationCompleted` invokes the reverse-hello callback
inline while the gate is held.

`ChannelGate` is now a `SemaphoreSlim(1,1)` behind a `Releaser`;
`Holder`, the thread-identity fallback, `LeaveInheritedContext`,
`IsHeldByCurrentContext` and all eight disclaimer calls are gone, with
the call shape at all 32 sites unchanged. `SaveIntermediateChunk`,
`GetSavedChunks` and `DoMessageLimitsExceeded` gained a `gateHeld`
argument, because the client reaches them both with and without the gate
and must pick the matching teardown — without it an oversized
`OpenSecureChannel` response deadlocked the receive loop against itself.

The remaining paths were not found by reading the code, since that is
what missed them originally. A temporary strict mode made every
re-entrant acquisition throw with its stack, the suite was run under it
until clean, and it was deleted with the re-entrancy it measured. It
caught two sites review had missed: the `EndpointDescription` **property
getter** taking the gate, and the `RequestReceived` dispatch to
application code happening inside the guarded region.

Two deliberate residuals are documented for reviewers: `Dispose` no
longer takes the gate (unavoidable — it is reached from `ChannelFaulted`
with the gate held, where the old re-entrant acquisition excluded
nothing anyway), and the deferred request dispatch builds
`SecureChannelContext` just after gate release rather than under it.

## Testing

New: the three seams and their platform implementations, facet discovery
and resolution precedence, `FipsOnly` refusing a provider that cannot
serve what it was bound to, AES counter mode (including a
non-block-aligned round trip and a check that the counter carries, since
one that did not would silently reuse key stream), `ChannelGate`
(exclusion, re-entrancy across an await, re-entrancy on a reused thread,
and the deadlocks above), the asynchronous channel paths, and the PubSub
policy provider.

Each review finding got a test that fails without its fix:

- `ContendedEntryStillRecordsTheHolderAsync` — established contention
deterministically, then re-entered; failed on the pre-fix gate. Removed
with the re-entrancy it covered; `ChannelGateTests` now asserts the
non-re-entrant contract instead.
- `SecureClientAndTcpListenerKeepChunkOrderUnderConcurrentRequestsAsync`
— 60 concurrent multi-chunk `SignAndEncrypt` requests over a real
loopback listener; failed 3 runs out of 3 with the previous dispatch,
passes 3 out of 3 with the chain.
-
`DependencyInjectionRoutesPubSubCryptoThroughTheRegisteredProviderAsync`
— builds a container, registers a counting provider, wraps a message and
asserts the provider actually performed the encryption and the
signature. This is the assertion the PubSub claim above now rests on.
- `PolicyScopedRandomBindingDoesNotEscapeItsPolicy`.

Two further assertions are the ones that would catch a silent defect: a
provider and the platform must produce **byte-identical** output — a
validated module that disagreed could not interoperate — and with a
software key the returned `ValueTask` must report
`IsCompletedSuccessfully`, which is what makes "ordering is unchanged"
checkable rather than merely asserted.

`SymmetricChannelCryptoBenchmarks` gains a provider-registered variant
beside the existing baseline, so the cost of the indirection is measured
rather than assumed.

Verified locally, every suite at or above its recorded baseline: `dotnet
build UA.slnx -c Release` **0 warnings, 0 errors on all six target
frameworks**; Core **4413** (net10.0); Core.Security **617**, with its
certificate-leak detector clean; PubSub **1350** plus PubSub.Diagnostics
**54** and PubSub.Udp **233**; Features **156**; Subscriptions.Classic
**32**; Server integration **4053**; Client integration **2123**;
MigrationAnalyzer **151** and MigrationAnalyzer.Core **13**. Zero
failures in each.

Core moved from 4388 to 4381 when the gate change landed — eight
re-entrancy tests were removed along with the behaviour they covered,
and one deadlock regression test added — and has since grown to 4413
with the registry isolation, provider-output and security-review tests.
`ChannelGateTests` was rewritten around the new contract — exclusion
across both entry modes, an uncontended asynchronous entry not
suspending, re-entry blocking by contract, and the property that used to
be the hazard: work started while the gate is held is excluded without
having to opt out.

Every CI failure this PR produced was reproduced locally, diagnosed
against a clean `origin/master` baseline in a second worktree, and fixed
at the source. Two hangs were found from `--blame-hang-timeout` dumps
read with `dotnet-dump`. One failure that looked like a candidate was
confirmed **not** to be: an intermittent `Opc.Ua.Aot.Tests` connect
failure, pre-existing on `origin/master` (5 of 8 baseline runs failed,
versus 3 of 14 here).

### The publish request overshoot

`Subscriptions.Classic.PublishRequestCountAsync` failed on the Windows
net10.0 leg with **207** outstanding publish requests against a cap of
`max(maxServerPublishRequest, subscriptions)` = 50. It failed in 2 of 6
pipeline builds on this branch, never on `master` in 13 sampled builds,
and never locally in 12 attempts including under deliberate CPU
contention.

The mechanism is a genuine race.
`ClassicSubscriptionEngine.QueueBeginPublish` was a **check-then-act**:
it read `GoodPublishRequestCount`, compared it against
`GetDesiredPublishRequestCount(false)`, and issued a publish if it was
below. It runs on every publish completion, so *n* completions that
overlap each read the same value, each conclude they are below the cap,
and each issue — overshooting by up to *n* per round. The read also
*lags*: a request only becomes visible to `GoodPublishRequestCount` once
`AsyncRequestStarted` has recorded it, which happens after the request
has already been sent.

That code is pre-existing and was untouched by this PR. What this PR
changed is how hard the race is hit: the server used to hold the channel
gate across request processing, which serialised every request on a
channel, so publish completions arrived one at a time. Dispatching
`RequestReceived` outside the gate — an intended improvement, since a
channel should not serialise on application request-processing time —
lets them complete concurrently.

Rather than change the publish pipeline blind, the race was first made
reproducible.
`ConcurrentPublishReEvaluationDoesNotExceedDesiredRequestCount` drives
64 barrier-released completions against a mocked context whose count
lags the send exactly the way the session's does. It issued **64
requests where 5 were desired** — the same defect the CI leg hit, now
deterministic and running in milliseconds.

The engine now tracks the requests it has in flight and reserves a slot
with a compare-exchange before sending, so the decision and the send are
one atomic step. The reservation is released when the request completes,
when the send is declined, or when it throws. The repro test then issues
at most the desired count.

`StartPublishing` is deliberately left **uncapped**. It is the recovery
valve that refills a pipeline whose requests are outstanding but no
longer expected to return, so gating it on the in-flight count would
remove the only path out of a stalled pipeline. An earlier revision of
this fix did gate it, and that is the one change here that could not be
justified on the evidence — it was reverted.

The assertion was **not** loosened, retried away, or marked flaky: it
caught a real throttling regression earlier in this PR and is worth
keeping sharp.

Validated: the repro test red before the fix and green after;
`Subscriptions.Classic` 32/32 **five** consecutive runs on net10.0 and
once on net48; `Opc.Ua.Client.Tests` 2124 on net10.0 across four
consecutive runs and 2127 on net48; `Opc.Ua.Subscriptions.Tests` 604
passed with this fix in place.

### The SSH.NET advisory that turned CI red repo-wide

Partway through this work every build job began failing with `NU1903:
Package 'SSH.NET' 2025.1.0 has a known high severity vulnerability`
([GHSA-q939-rpr3-3284](https://github.com/advisories/GHSA-q939-rpr3-3284),
`<= 2025.1.0` affected, first patched `2026.0.0`).

Nothing in the dependency graph changed — the advisory was published.
Build 17026 failed on it where build 17000, two hours earlier on an
identical graph, had passed every build job. SSH.NET is not referenced
anywhere in the repository; it arrives transitively through
`Testcontainers.Kafka`.

`master` landed the remediation independently while this was being
investigated (a central transitive pin to `2026.0.0`, which
`CentralPackageTransitivePinningEnabled` exists for). This branch
briefly carried its own identical pin, which made the merge commit hold
**two** `PackageVersion` entries for SSH.NET and fail restore with
`NU1506`. The duplicate is removed and `master` is merged, so the pin
now comes from one place only.

The audit was **not** suppressed: `NuGetAudit` stays enabled and
`NU1903` stays an error. Verified after the merge — `dotnet restore
UA.slnx` reports neither NU1903 nor NU1506, and `dotnet build UA.slnx -c
Release` is **0 warnings, 0 errors** with the audit live.

The remaining red check in build 17026 was not from this change either.
`aot-ubuntu-latest` is the intermittent `Opc.Ua.Aot.Tests` failure
already noted above — `ReconnectSessionAsync` timed out at 30s with 135
of 136 passing. It remains below the `origin/master` baseline rate (4 of
15 sampled here against 5 of 8 on the baseline), and the managed
reconnect paths are green throughout: `Opc.Ua.Client.Tests` 2124 across
four consecutive runs and `Opc.Ua.Subscriptions.Tests` 604.


## Documentation

`docs/CryptoProvider.md` gains sections on substituting the symmetric
primitives, using a key served over a network, and PubSub — including
what cannot be claimed and why, the scope rule for a random-source
binding, and how to supply a provider-backed policy outside the
container. `docs/MigrationGuide.md` covers the `[Obsolete]` channel
virtuals and `DataLock`, and the non-re-entrant gate's contract for
channel subclasses. `docs/WhatsNewIn2.0.md` is updated.
`plans/cryptooffboard.md` is deleted now that the work it planned has
landed.

## Note for reviewers

This touches the secure channel handshake, which is the most
security-critical code in the stack. Nine distinct concurrency and
lifetime defects were found and fixed in it during this work — three
during development, two in review, four from CI — so `ChannelGate`, the
write path and the certificate ownership across the new suspension
points deserve the closest reading.

Five of those nine traced to the gate's `AsyncLocal`, which is why the
re-entrancy was subsequently removed and the `AsyncLocal` deleted. The
wrapper/`Core` split and the two documented residuals in that section
are the parts most worth a second reading.

The #4206 work removes the last reflection from `Security/Constants` and
rewrites the policy lookup tables, so
`SecurityConfiguration.SupportedSecurityPolicies` and the endpoint set
it feeds are worth a second look: an earlier revision of it changed that
set and broke six server tests, which is now covered by
`SecurityConfigurationBuildsExpectedSupportedPolicySet`.

## Related Issues

- Fixes #4206
- Fixes #4207
- Fixes #4208
- Fixes #4210
- Follow-up #4250 — this PR makes the security policy set injectable,
but nothing in the stack resolves `ISecurityPolicyRegistry` from DI yet,
so every production path still lands on the `Default` fallback. #4250
tracks threading it through `Session` and `UaSCBinaryChannel`, which is
what makes the injection points already added here load-bearing.

## 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.
- [x] 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 <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Copilot-Session: 7a68ca72-83e8-496c-b2f5-3df4c718320b
Copilot-Session: 3f5ab76a-c0ba-4f47-87ac-54d1f4182c6b
 2026-08-15 05:23:20 -07:00
MrAlaskan 8f4087605a Fix unordered server endpoint validation (#4029) 
### Summary

This PR makes `Session.ValidateServerEndpoints()` compare
`ServerEndpoints` and `UserIdentityTokens` as unordered sets and adds
regression tests that verify `OpenAsync()` no longer rejects legal
servers that return the same endpoint data in a different order.

### Problem

According to OPC UA Part 4, the client validates the endpoint set
returned by `CreateSessionResponse.ServerEndpoints` against the endpoint
set observed during discovery. The specification describes these values
as a set of endpoint descriptions filtered by the relevant transport
profile, not as an ordered array that must preserve the exact
discovery-time enumeration order.

Previously, `Session.ValidateServerEndpoints()` first checked the
endpoint count and then compared `m_discoveryServerEndpoints[ii]`
against `serverEndpoints[ii]` by index. It also compared
`UserIdentityTokens[jj]` by index within each endpoint. As a result, a
server that returned the same legal endpoints in a different order, or
returned the same token policies in a different order within an
endpoint, could be rejected with `BadSecurityChecksFailed` even though
the endpoint data itself had not changed.

### Changes

- Replace index-based `ServerEndpoints` validation with unordered
matching based on the same endpoint fields already validated by the
client.
- Compare `UserIdentityTokens` as an unordered multiset instead of
requiring the server to preserve the original token enumeration order.
- Extend the client session test scaffolding so discovery endpoints can
be supplied explicitly.
- Add regression tests that verify `OpenAsync()` accepts reordered
`ServerEndpoints` and reordered `UserIdentityTokens`.
 2026-07-20 08:17:34 +02:00
MrAlaskan 12f770b3b6 Fix managed session failover target selection (#4025) 
### Summary

This PR makes `ManagedSession` select failover targets relative to the
currently active endpoint after a successful failover and adds a
regression test that verifies subsequent failover decisions no longer
use the original connection target as the current server.

### Problem

According to OPC UA Part 4 redundancy semantics, once a client has
failed over to a different redundant server, subsequent target selection
must be based on the server that is currently hosting the active
session. The current server is not permanently tied to the endpoint that
was used for the initial connection.

Previously, `ManagedSession` stored the constructor-supplied
`ConfiguredEndpoint` in a read-only property and always passed that
outer value into `IServerRedundancyHandler.SelectFailoverTarget()`.
However, the inner `Session` updates its own `ConfiguredEndpoint` during
`RecreateInPlaceAsync()` when failover succeeds. After the first
failover, the outer `ManagedSession` and inner `Session` could therefore
disagree about which redundant member was currently active, causing
later failover decisions to be evaluated against the original endpoint
instead of the active one.

### Changes

- Make `ManagedSession.HandleFailoverAsync()` prefer the inner session's
current `ConfiguredEndpoint` when selecting a failover target, while
keeping the outer endpoint as a fallback before the session is
available.
- Preserve the existing failover flow and scope the fix to target
selection only.
- Add a regression test that verifies `HandleFailoverAsync()` passes the
inner session's current endpoint to
`IServerRedundancyHandler.SelectFailoverTarget()`.
 2026-07-19 07:43:19 +02:00
Marc Schier 12ebfba603 Add ISA95 JobControl and ManagedSession regression coverage (#3998) 
## Summary

- Adds the official ISA95 JobControl 2.0.0 NodeSet2 as a complete
source-generator regression fixture.
- Extends the lightweight generated-stack test stub with
`NamespaceMetadataState`, required by the complete model.
- Adds a deterministic ManagedSession cancellation regression proving
that a started connect attempt is stopped.

The ManagedSession cleanup implementation and Quickstarts utility
coverage now come from `master`; this PR retains the additional
deterministic cleanup regression.

## ISA95 fixture provenance and integrity

The fixture is byte-for-byte the official file:

- Source:
`OPCFoundation/UA-Nodeset@e0b5d80cfff698f0276393e28df138a7fbc4b541`
- Path: `ISA95-JOBCONTROL/opc.ua.isa95-jobcontrol.nodeset2.xml`
- Git blob: `82f008cd126ade54e5f2c737bcc116621a8c3087`
- SHA-256:
`52327e736beb604c7253a5a5393d1ad525b9ecf35b71fbacb4f429cacb334064`

The fixture test retains the OPC Foundation MIT header and verifies the
2.0.0 model/date, 258 nodes (including 134 variables), representative
late variables, and that every `ns=1` reference target resolves locally.

## Validation

- ManagedSession tests: 17 passing on net10.0; 15 passing and 2
framework-gated skips on net48.
- Quickstarts server utility tests: 3 passing on net10.0 and net48.
- ISA95 generator and fixture integrity regression: 5 passing on
net10.0.
 2026-07-17 13:19:00 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00