GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-18 14:43:08 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,540 Commits 101 Branches 105 Tags
aas
Commit Graph

2540 Commits

This Branch
This Branch
All Branches
Author SHA1 Message Date
romanett 414957c88a Reduce ISubscription: internalize the publish pipeline, delete the dead members (#4349) 
# Description

Reduces the server subscription API surface per the measurements from
issue #4185.

## ISubscription: 42 → 30 members (#4185)

A call-site census of the 11 "publish-pipeline protocol" members plus
`Publish` (all 68 `src/` projects, samples, tools, fuzzing, tests) found
**zero callers outside `Opc.Ua.Server`** and split the members three
ways:

- **Deleted (4):** `ItemReadyToPublish` and `ItemNotificationsAvailable`
were dead — zero callers repo-wide and implementation bodies commented
out since before 1.5.378 — the obsolete parameterless `SessionClosed()`
had zero callers, and `TransferSessionAsync` (the one-shot direct
transfer, which bypassed the reservation protecting a transfer against a
concurrently closing source session) lost its last caller once the
manager always uses the claim/prepare/commit protocol.
- **Internalized (8):** `PublishTimerExpired`, `Acknowledge`,
`PublishTimeout`, `SubscriptionTransferred`,
`AvailableSequenceNumbersForRetransmission`, `QueueOverflowHandler`,
`SessionClosed(ISession)` and `Publish` move to `internal interface
ISubscriptionPublishPipeline : ISubscription`, implemented
**explicitly** by `Subscription` (the
`ISubscriptionMonitoredItemLifecycle` shape) so they are unreachable
through the public concrete class too. `Publish` goes with them
deliberately — it is the same hazard class (consumes notifications,
advances sequence numbers) with the same single production caller.
- **Kept public (1):** `ResendData` drops out of the issue's list — its
only caller is the `Server_ResendData` UA-method handler in
`DiagnosticsNodeManager`, resolving via `TryGetSubscription` like
`GetMonitoredItems` does. It is service-facing, not pipeline.

## The pipeline contract is complete: SessionPublishQueue traffics only
in it

`SessionPublishQueue` becomes `internal sealed` (zero consumers outside
the assembly; `SentMessageQueue` already was) and works **solely with
`ISubscriptionPublishPipeline`** — no `ISubscription` and no concrete
`Subscription` anywhere in the file. To get there, the internal
transfer-claim protocol (`TryBeginTransfer`,
`PrepareSessionTransferAsync`, `CompleteTransfer`, `AbortTransfer`,
`TryRestoreSessionAfterFailedTransfer`) moved from `internal` methods on
the concrete class onto the internal interface, implemented explicitly.

That completes the contract, so `TransferSubscriptionsAsync` loses its
two-tier shape: the ~6 `subscription as Subscription` null-guard
branches and the degraded `TransferSessionAsync` fallback for
non-`Subscription` implementations are gone — one code path, always the
claim/prepare/commit/rollback protocol, and
`ISubscription.TransferSessionAsync` is deleted with it. Subscription
creation fails with `Bad_InternalError` when a `CreateSubscription`
override returns a type not derived from `Subscription`, instead of
publishing partially — such implementations were already second-class
(transfer required the concrete type before this change). This is the
headline behavior change for exotic extenders and is documented
prominently in `docs/migrate/2.0.x/sessions-subscriptions.md`.

The durable-subscription open question in the issue is answered **no**:
`Subscription/Persistence/` never references `ISubscription` (the store
deals in `IStoredSubscription` DTOs; restore constructs the concrete
type via `Subscription.RestoreAsync` inside the manager), so the
internalization cannot break a store implementer.

## #4186: measured, prototyped, and deliberately kept as-is

Body inspection showed only 4 of the 10 `ISubscriptionManager` wrappers
are pure lookup-and-delegate (`Republish`, `SetTriggering`,
`ModifyMonitoredItemsAsync`, `SetMonitoringModeAsync` — one caller
each); the other 6 carry logic with no `ISubscription` substitute. A
collapse of the four was implemented and then reverted on this branch:
it traded ten uniform id-addressed members for a mixed dispatch shape in
`StandardServer` without reducing what a reader must understand. **Every
subscription service keeps going through `ISubscriptionManager`**; the
wrapper bodies now resolve against the pipeline-typed registry. The
decision is recorded in `plans/ServerRuntimeAnalysis.md` (open question
5).

## Compatibility treatment

Removals are outright, per the documented no-`[Obsolete]`-shim policy
for these interfaces (`docs/MigrationGuide.md` — re-adding an interface
member would break every implementer; net48/netstandard2.0 have no
default interface methods). In its place:

- Migration analyzer **UA0030** (pipeline members +
`SessionPublishQueue`, distinguishing deleted no-ops from internalized
protocol), with tests, following the UA0024 syntax-based pattern. The
transfer-claim members were never public in 1.5.378, so they need no
migration story.
- `docs/MigrationGuide.md#ua0030` section with before/after code; the
`docs/migrate/2.0.x/sessions-subscriptions.md` section that told
implementers to *add* `SessionClosed(ISession)` is replaced with the
derive-from-`Subscription` story; the 2.0.x README symptom table routes
the rule.
- All 11 removed/internalized members existed in 1.5.378 (verified
against `master378`), so each is documented as a 2.0 break.

## Tests

Converted, not deleted. `Mock<ISubscription>` setups of pipeline members
become `Mock<ISubscriptionPublishPipeline>` — Moq proxies the internal
interface via a new `InternalsVisibleTo("DynamicProxyGenAssembly2")`
(keyless; the SDK appends `$(PublicKey)` under `SignAssembly`, and
signed pipelines do not run tests, matching the existing
`Opc.Ua.Redundancy.Kubernetes` precedent). A test-side extension shim
forwards the original member names (pipeline + transfer protocol) so the
direct call sites on concrete `Subscription` instances compile
unchanged. The transfer-race tests' typed reflection probes into
`m_abandonedSubscriptions` follow the field's new type.

- `Opc.Ua.Server.Tests` (net10.0): 4795 tests, 0 failures
- `Opc.Ua.Subscriptions.Tests`, `Opc.Ua.Subscriptions.Durable.Tests`,
`Opc.Ua.Subscriptions.Classic.Tests` (net10.0): green
- `Opc.Ua.MigrationAnalyzer.Tests`: green incl. the new UA0030 tests
- net48 gate: solution build green; subscription-area fixtures green
(the known pre-existing local hosting-test environment failures
reproduce identically on unmodified master)


## Related Issues

- Fixes #4185
- #4186 was measured and prototyped on this branch and deliberately
**not** taken (see the section above); the decision is recorded in
`plans/ServerRuntimeAnalysis.md`.

## 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. _(net10
suites fully green locally; net48 build green and subscription-area
fixtures green — the remaining net48 hosting-test failures on my machine
reproduce identically on unmodified master.)_
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-31 10:53:51 +00:00
romanett 2bd4294863 [Server] Decouple service-call dispatch from MasterNodeManager lifecycle coordination (#4345) 
# Description

Extracts OPC UA service-call dispatch out of `MasterNodeManager` into a
dedicated internal `NodeManagerServiceDispatcher`, separating it from
NodeManager lifecycle coordination as agreed in the #4094 review threads
(r3649968439 / r3649998212). The 7,668-line class becomes four files
with explicit, documented boundary contracts:

- **`NodeManagerServiceDispatcher.cs`** (new, internal sealed) — the
implementations of GetManagerHandle/Async, TranslateBrowsePaths,
Browse/BrowseNext, Read, Write, Call, HistoryRead/HistoryUpdate,
ConditionRefresh, all monitored-item operations
(create/restore/modify/transfer/delete/set-mode), and per-request
validation. Its constructor receives only the routing table, the server
context, and a logger. Lifecycle mutation state
(`m_dynamicMutationSemaphore`, `m_startupShutdownSemaphoreSlim`,
retired-generation collections, preparing-manager tracking) stays
`private` to `MasterNodeManager`, so the dispatcher cannot reach it —
the boundary is enforced by the compiler, not convention.
- **`MasterNodeManager.cs`** — now lifecycle coordination and shared
infrastructure only: startup/shutdown, session notifications, namespace
registration, all `IDynamicNodeManagerHost` /
`ISyncNodeManagerMonitoredItemRecovery` implementations, the
retired-generation notification-lease machinery, and
commit/replace/rollback helpers.
- **`MasterNodeManager.ServiceDispatch.cs`** (new partial) — the
unchanged public service surface. Every public virtual entry point,
protected helper, static validator, and obsolete wrapper keeps its exact
signature, virtual-ness, and XML docs, delegating to the dispatcher.
- **`MasterNodeManager.NodeManagement.cs`** (new partial) — the
AddNodes/DeleteNodes/AddReferences/DeleteReferences service family plus
NodeId-based reference plumbing. These sit between the two concerns by
design: they dispatch per item but serialize address-space mutation
against runtime lifecycle operations, and are the only service methods
allowed to take the dynamic-mutation semaphore.

The dispatcher's crossings back into `MasterNodeManager` are a small
audited set: internal dispatch resolves handles through the owner's
public virtual `GetManagerHandleAsync` (10 sites), so derived-class
overrides keep affecting internal routing exactly as before; event paths
consume the notification-dispatch lease accessors (5 sites, widened to
internal); retired-generation drain is signalled through a new one-line
internal helper instead of touching the observer field (2 sites); and
the browse continuation-point limit is read live per call (2 sites).

## Compatibility

- `IMasterNodeManager` is untouched; no public or protected member
changes signature or loses `virtual`.
- The `MasterNodeManagerWithLimits` test-framework subclass (overrides
`BrowseAsync`, uses `ValidatePermissionsAsync`, the per-item
`BrowseAsync`, `UpdateDiagnostics`, `Server`) passes unchanged.
- Reflection-dependent tests keep working:
`m_maxContinuationPointsPerBrowse` remains a private field on the type
and is read live on every call; `PreHydrateMonitoredItemQueuesAsync`
remains as a private delegating method.
- Reviewable API-surface deltas, both additive: six `protected static`
validators became `protected internal static` (same pattern as the
existing `ValidateRolePermissions`) so the dispatcher can call them, and
the three notification-lease nested types went `private` → `internal`.

Reviewing tip: `git diff --color-moved=dimmed-zebra` renders the two
move commits as almost entirely dimmed blocks; the non-move edits are
confined to the audited call sites above and the delegation stubs.
Grepping the dispatcher for
`IDynamicNodeManagerHost|m_dynamicMutationSemaphore|m_startupShutdownSemaphoreSlim|m_retiredGeneration|m_preparingNodeManagers`
matches only its own boundary-contract doc comment.

## Verification

- Full `UA.slnx` build with 0 errors; `Opc.Ua.Server` itself builds with
0 warnings.
- Full `Opc.Ua.Server.Tests` on net10.0: 4707 passed, 0 failed (5
pre-existing skips).
- Targeted: MasterNodeManager/Deterministic/NodeManagement 189/189;
NodeManagerLifecycle/MonitoredItemLifecycle/MonitoredItem 406/406;
`ContinuationPointInBatch` (subclass contract) 28/28.
- `docs/NodeManagers.md` documents the new internal split.

## Related Issues

- Fixes #4098

## 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.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage. _(pure refactoring; behavior
is pinned by the existing MasterNodeManager, deterministic,
node-management, lifecycle, and monitored-item suites, which all pass
unchanged)_
- [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. _(full
server suite green on net10.0; .NET Framework run pending CI)_
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
 2026-08-31 12:04:47 +02: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
romanett 14fba2d120 Fix instance TypeDefinition across dependency ModelDesign boundaries (#4355) 
# Description

The model source generator crashed with `MODELGEN003` /
`NullReferenceException` whenever a `ModelDesign` instantiated an
`ObjectType` (or `VariableType`) declared in a **dependency** design
file, as soon as that type had children. The sibling of #4332 on the
instance-declaration path: #4335 linked dependency *data types*, but an
*instance* whose `TypeDefinition` lives in another design still crashed
in `NodeStateGenerator` on the type's inherited children.

Root cause: dependency design files load with `validateDictionary:
false`, so `ValidateInstance` / `ValidateType` never ran on them and
their types' children kept `TypeDefinitionNode` / `DataTypeNode` at
`null`. When the target instance merged those children into its
hierarchy (`BuildInstanceHierarchy` re-walks the dependency type's
`Children.Items`), the null references were copied along, and
`GetNodeStateClassName` dereferenced them while building the node state
factory list.

Changes:

- A new best-effort `LinkDependencyInstances` pass in
`ModelDesignValidator` links dependency-design instances the way
`ValidateInstance` / `ValidateType` do for the target model: the
`TypeDefinitionNode` / `DataTypeNode` of the types' children and
standalone instances (recursively), variable `DataType` / `ValueRank`
inheritance from the variable type, `DefaultValue` decoding,
`VariableTypeDesign.DataTypeNode` restrictions, and method arguments —
including creation of the `InputArguments` / `OutputArguments` argument
properties so method children materialise into the target hierarchy
exactly like local ones. Resolution is best effort (unresolvable
references stay `null`) and NodeSet2 dependencies remain excluded,
consistent with `LinkDependencyDataTypes`.
- The pass runs alongside `LinkDependencyDataTypes`: for upstream
dependencies after all of them are loaded and before the target
validates (so the hierarchy merge picks up the links), and for
downstream dependencies after they load.
- `docs/ModelDependencies.md` documents the instance linking.

Tests (all verified to fail with the exact reported NRE stack before the
fix):

- `CrossModelInstanceTypeDefinitionTests` (Core, 4 tests): the issue's
minimal repro end to end; an equivalence test asserting the cross-model
instance emits the **identical node-state factory set** as a locally
declared type (this is the issue's stated expectation, and it drove the
method-argument-property part of the fix); the flipped
upstream/downstream ordering; and validator-level assertions that the
merged hierarchy carries linked `TypeDefinitionNode` / `DataTypeNode`
references, including the data type inherited from a dependency
`VariableType`.
- `CrossModelInstanceTypeDefinitionGeneratorTests` (Roslyn driver, 2
tests): the issue's csproj repro shape — both `AdditionalFiles`
orderings, asserting no diagnostics and that the combined generated
output of both models **compiles** (the instance factories reference the
dependency model's typed state classes).

Verified locally on net10.0: `Opc.Ua.SourceGeneration.Core.Tests` 3800
passed, `Opc.Ua.SourceGeneration.Tests` 141 passed,
`Opc.Ua.SourceGeneration.Stack.Tests` 94 passed; 0-warning Release build
of `Opc.Ua.SourceGeneration.Core` (all TFMs incl. netstandard2.0);
`dotnet format whitespace --verify-no-changes` clean.

This removes the last generator blocker for source generating the
instance model of the `Workshop/DataTypes` sample
(OPCFoundation/UA-.NETStandard-Samples#814): its `ParkingLot` /
`DriverOfTheMonth` are instances of types declared in the other design
file.

## Related Issues

- Fixes #4353

## Checklist

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

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

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
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-08-31 09:47:08 +00:00
romanett 24ba4855e6 Fix [DataType] generator Clone() dropping every inherited field of a derived structure (#4354) 
# Description

The `[DataType]` source generator emitted a `Clone()` that created a
fresh instance and copied only the fields declared on the annotated type
itself, so every field inherited from an `IEncodeable` base was silently
dropped. Since the server clones node values while loading an address
space, a derived structure served by a node manager came back out of a
Read with its inherited fields empty, even though encode/decode was
correct.

This PR makes the `[DataType]` path emit the same
`MemberwiseClone`-based shape as the model-based generator:

```csharp
/// <inheritdoc/>
public override object Clone()
{
    return (DerivedStruct)this.MemberwiseClone();
}

/// <inheritdoc/>
public new object MemberwiseClone()
{
    DerivedStruct clone = (DerivedStruct)base.MemberwiseClone();
    // deep copy of the own reference-typed fields
    return clone;
}
```

`Clone()` delegates to a generated `public new object MemberwiseClone()`
that starts from `base.MemberwiseClone()` — a shallow copy of the whole
object, inherited fields included — and then deep copies the type's own
reference-typed fields (encodeables, `DataValue`, `Variant`,
`ExtensionObject`). Derived classes hide `MemberwiseClone` and chain
into the base implementation, so deep-copy semantics compose across the
hierarchy, including across assemblies into model-generated base types
(the `BicycleType` scenario that surfaced this). The two generator paths
now agree.

Because the clone now starts from a full shallow copy
(`base.MemberwiseClone()` for classes, `with { }` for records), the
per-field clone emitter only writes the deep-copy assignments; the
redundant per-field plain copies are gone. This also fixes a latent side
bug in the class path: init-only simple fields were silently dropped by
the fresh-instance clone as well.

Records were already correct (`with { }` carries every field) and are
unchanged in shape.

Tests:

- `GenerateClassCloneUsesMemberwiseClone` /
`GenerateDerivedClassCloneCarriesInheritedState` assert the emitted
shape for root and derived classes.
- `DerivedClassCloneKeepsInheritedFields` runs the exact repro from the
issue through the Roslyn driver, emits the assembly, and executes
`Clone()` at runtime to prove the inherited field survives (verified on
net10.0 and net48).

Verified locally: `Opc.Ua.SourceGeneration.Core.Tests` (3798 passed),
`Opc.Ua.SourceGeneration.Tests` (140 passed),
`Opc.Ua.SourceGeneration.Stack.Tests` (94 passed), and
`Opc.Ua.Client.Tests` (2161 passed, exercises the regenerated in-tree
`[DataType]` record types). The in-tree `[DataType]` consumers
(Opc.Ua.Client, both GDS common projects) regenerate and build without
warnings.

## Related Issues

- Fixes #4352

## Checklist

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

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

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

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-30 22:10:27 +00:00
romanett 16c54e52d9 Keep application certificates which do not name a CertificateType (#4348) 
# Description

`SecurityConfiguration.ApplicationCertificates`'s setter removed every
identifier whose certificate type is not supported, and
`CertificateIdentifier.CertificateType` defaults to unset. An otherwise
correct identifier passed to the recommended
`AddSecurityConfiguration(ArrayOf<CertificateIdentifier> certIdList,
...)` overload was therefore dropped silently, and validation then
failed with `ApplicationCertificate must be specified.` — pointing at
the wrong cause. The obsolete `AddSecurityConfiguration(string
subjectName, ...)` overload escaped this because it assigns the legacy
singular `ApplicationCertificate` property, whose setter fills in
`RsaSha256ApplicationCertificateType` itself. The result: the
replacement advertised by the `[Obsolete]` message could not be used
without also setting `CertificateType`, which that message does not
mention.

**Changes:**

- `SecurityConfiguration.ApplicationCertificates`
(`src/Opc.Ua.Core/Schema/ApplicationConfiguration.cs`) now treats an
unset `CertificateType` as the default RSA-SHA256 application
certificate type instead of dropping the entry — the same assumption the
legacy single `ApplicationCertificate` element carries, so both paths
agree on what a valid `CertificateIdentifier` is. Fixing the setter also
covers callers that assign `ApplicationCertificates` directly, not just
the builder overload.
- Identifiers dropped for a genuinely unsupported type are no longer
discarded without a trace. They are remembered and reported by
`SecurityConfiguration.Validate`, which has an `ITelemetryContext` —
this replaces the `// TODO: Log when ITelemetry instance is available`
that was exactly this bug:
- a warning naming the rejected certificate types is logged whenever any
were filtered out;
- when the collection ends up empty *because* of the filtering, the
error now reads `No supported application certificate configured: N
certificate identifier(s) were rejected because their CertificateType is
not supported (...)` instead of `ApplicationCertificate must be
specified.`. The original message is kept for the genuinely-empty case.

**Notes for reviewers:**

- Behaviour is unchanged for every configuration that already names a
certificate type; all shipped and test config XMLs set
`CertificateTypeString`.
- `FindApplicationCertificateAsync` already had a fallback treating a
null certificate type as RsaSha256, and `ApplicationInstance` fills in
the type from the created/validated certificate when it is null —
defaulting in the setter is consistent with both and yields the same RSA
certificate.
- The unsupported-type filter itself is untouched; only unset types are
now kept.

## Related Issues

- Fixes #4315

## 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.
- `SecurityConfigurationTests`: an untyped identifier survives as
RsaSha256 and contributes `Basic256Sha256` to
`SupportedSecurityPolicies`; an unsupported type
(`EccCurve25519ApplicationCertificateType`) is removed and reported by
`Validate` with `BadConfigurationError`; a supported entry survives
alongside an unsupported sibling without failing validation.
- `ApplicationConfigurationBuilderTests`: end-to-end reproduction from
the issue, building a client through
`AddSecurityConfiguration(ArrayOf<CertificateIdentifier>, pkiRoot)` +
`CreateAsync()`.
- [ ] 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.
- Ran on net10.0: `Opc.Ua.Configuration.Tests` 225/225 passed,
`Opc.Ua.Core.Tests` 4368 passed / 86 skipped / 0 failed. The full
solution across .NET Framework was not run locally — relying on CI for
that.
- [ ] 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-30 20:27:20 +00:00
romanett 52df519e50 Expose the client configuration provider and add an opt-in eager load on host start (#4343) 
# Description

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

Two small additions, exactly as designed in #4340:

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

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

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

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

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

A WinForms client then becomes:

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

await host.StartAsync();

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

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

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

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

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

## Related Issues

- Fixes #4340
- Builds on #4328

## Checklist

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

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

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

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
 2026-08-30 20:50:46 +02:00
romanett ad6430f560 Fix transient PublishWorkerCount overshoot in the publish controller (#4344) 
Fixes #4339.

## Cause

`PublishControllerAsync` sized the worker pool at the **top** of its
loop, before its first `await`. The `SubscriptionManager` constructor
calls the controller directly, so that first resize ran *synchronously
inside the constructor* — with
`MinPublishWorkerCount`/`MaxPublishWorkerCount` still at their defaults
(2 / 15), because owners configure them afterwards.

A session that already reports subscriptions — a classic, session-owned
subscription counts through
`ISubscriptionManagerContext.SessionSubscriptionCount` — therefore had
`1` clamped **up** to the default minimum of 2, spun up two workers in
the constructor, and published that overshoot through
`PublishWorkerCount`. The configured `Min = Max = 1` then shrank the
pool back asynchronously.

Confirmed with a probe before changing anything:

```
PROBE right after ctor: 2
```

which reproduces the issue's `Worker #0 STARTED / #0 PAUSED / #1
STARTED` log exactly.
`ClassicSessionSubscriptionStartsPublishWorkerAsync` pins the pool to
one worker and samples the property directly, so it failed whenever it
read it before the controller had shrunk the pool back.

## Fix

The control loop now waits first and sizes the pool afterwards:

```
wait -> reap exited workers -> resize -> publish the count
```

The pool is only ever sized in response to a signal or a worker exit,
never from unconfigured defaults, and the count is assigned once the
pool has settled — so a worker that has exited is never counted
alongside its replacement (the second half of the issue's *Expected
Behavior*).

Two supporting changes were required:

- **Publish workers now start on the thread pool.** The controller
creates them inline, and the worker loop only yields where it awaits
something not already complete. With the resize moved behind the wait,
workers are typically created while publishing is already running, which
made that starvation reachable: I hit it while testing — 430k publishes
in 2 s with `PublishControlCycles` stuck at 1 and `PublishWorkerCount`
stuck at 0. The same starvation would hang `DisposeAsync`, which awaits
the controller.
- **`Resume()`, and leaving publishing quiescence, now signal the
publish control**, so the pool is sized promptly once publishing starts
rather than waiting for an unrelated event.

## Tests

- `ClassicSessionSubscriptionStartsPublishWorkerAsync` now settles the
pool through the existing `WaitForPublishWorkerCountAsync` helper
instead of sampling the property bare.
- New
`PublishWorkerPoolIsNeverSizedFromDefaultsBeforeConfigurationAsync` pins
the constructor behaviour (count stays 0 while the defaults are in
force) and asserts the count never leaves 1 while workers publish.

## Verification

- `Opc.Ua.Client.Tests`: 2151/2151 pass.
- `Opc.Ua.Subscriptions.Tests`: 619 passed, 6 skipped, 0 failed (22 m 18
s).
- The two affected tests stressed 15x in a row: green.
- `dotnet format whitespace --verify-no-changes` clean.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
 2026-08-30 17:40:48 +00:00
romanett 766c5558e8 Fix structure subtyping across dependency ModelDesign boundaries (#4335) 
# Description

The model source generator crashed whenever a `ModelDesign` declared a
structure that subtypes a structure from **another** design file
(`MODELGEN003` with a `NullReferenceException`), and the cross-assembly
`[assembly: ModelDependency]` flow failed even earlier with `The
BaseType reference ... is not the expected type: DataTypeDesign`.
Structure inheritance across model boundaries — the primary use case of
companion-spec layering — could not be source-generated at all.

Three root causes, all in the dependency-loading path of the validator:

1. **Dependency design files were never linked.** They load with
`validateDictionary: false`, so their `DataTypeDesign`s kept
`BasicDataType` at the enum default (`Boolean`) and their fields'
`DataTypeNode` stayed `null`. `BinarySchemaGenerator`'s inherited-field
merge then dereferenced the null `DataTypeNode`, and
`XmlSchemaGenerator` / `DataTypeGenerator` silently emitted `xs:boolean`
/ `bool` from the default classification.
2. **Dependencies that import the target crashed at load.** Each
generation group receives every design file of the compilation as a
dependency, including downstream models that import the group's target —
those loaded *before* the target they import and failed resolving their
`BaseType`. This produced the confusing
`AdditionalFiles`-order-dependent failure modes.
3. **The payload flow never reached ModelDesign targets.**
`ModelCompilation` supplied `referencedDependencies` only to the
NodeSet2 pass, never to the design-file pass, and payload-materialised
data types lacked the classification and field links needed to serve as
base types of local structures.

Changes:

- `ModelDesignValidator.ValidateModel` loads dependencies in two phases
around the target: upstream models first (topological, referenced models
before their referrers), then the target, then models that import the
target.
- A new `LinkDependencyDataTypes` pass classifies dependency data types
(`IsStructure` / `IsEnumeration` / `BasicDataType`) and resolves field
`DataTypeNode` / `Parent` best effort. It runs for payload-imported
declarations and upstream design-file dependencies before target
validation, and for downstream dependencies after they load. NodeSet2
dependencies are excluded — `NodeSetToModelDesign` already links and
classifies them with its own semantics.
- Payloads for models that are backed by a design file in the
compilation are skipped — the explicit file is authoritative
(previously, importing both crashed with duplicate symbolic ids).
- Payload-materialised data types honor the payload's `IsEnumeration`
flag.
- The design-file `GenerateCode` pass now receives the referenced
assemblies' dependency payloads (`ModelCompilation` + new optional
`referencedDependencies` parameter).
- `docs/ModelDependencies.md` updated accordingly.

Tests (all verified to fail with the exact reported errors before the
fix):

- `CrossModelStructureSubtypeTests` (Core, 6 tests): the issue's minimal
repro end to end — asserting the emitted `.bsd` carries inherited fields
with correct types and `SourceType`, the `.xsd` extends the dependency
structure (not `xs:boolean`), and the generated C# derives correctly —
plus the reversed target/dependency order, both payload flows at
generation and validator level, and a three-level chain (target →
payload structure → design-file structure).
- `CrossModelStructureSubtypeGeneratorTests` (Roslyn driver, 3 tests):
both `AdditionalFiles` orderings compiling the generated output, and the
full cross-assembly flow (producer assembly generated from the upstream
design; consumer resolves the base structure from the reference's
payload without re-emitting it).

Verified locally on net10.0: `Opc.Ua.SourceGeneration.Core.Tests` 3786
passed, `Opc.Ua.SourceGeneration.Tests` 135 passed,
`Opc.Ua.SourceGeneration.Stack.Tests` 94 passed; 0-warning Release
builds of `Opc.Ua.SourceGeneration.Core` (all TFMs incl.
netstandard2.0), both Roslyn generator variants, Stack and Tester.

This unblocks the `[NodeManager]` migration of the DataTypes workshop
sample (OPCFoundation/UA-.NETStandard-Samples#757).

## Related Issues

- Fixes #4332

## Checklist

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

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

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
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>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-30 17:22:34 +00:00
romanett 1f0125c385 Fix reverse connect hold time when several Servers share one listener (#4342) 
# Description

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

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

## Root cause

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

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

## Changes

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

No public API change.

## Validation

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

## Related Issues

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

## Checklist

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

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-30 16:38:07 +00:00
Copilot 90a85eda13 Fix V2 managed subscription keep-alive timer clamping causing false PublishingStopped flapping (#4324) 
# Description

V2 managed subscriptions were clamping computed keep-alive intervals to
`Timeout.InfiniteTimeSpan` (`-1ms`), then immediately flooring to `1s`,
which caused continuous false `PublishState.Stopped → Recovered`
flapping during normal publish gaps.
This change corrects interval clamping so the watchdog uses
server-revised keep-alive timing instead of a forced 1-second cadence.

- **Root cause correction (V2 `Subscription.StartKeepAliveTimer`)**
- Replaced invalid upper-bound check against `Timeout.InfiniteTimeSpan`
with a real timer max bound.
- Added `s_maxKeepAliveTimerInterval =
TimeSpan.FromMilliseconds(uint.MaxValue - 1u)` and clamp to that value.

- **Behavioral impact**
- Preserves expected keep-alive windows derived from
`CurrentPublishingInterval * (CurrentKeepAliveCount + 1)`.
  - Prevents false outage signaling on normal inter-publish gaps.

- **Regression coverage**
- Added a focused test asserting revised values (e.g., `2800ms`,
keep-alive count `30`) produce the expected interval and do not collapse
to `1s`.

```csharp
if (m_keepAliveInterval > s_maxKeepAliveTimerInterval)
{
    m_keepAliveInterval = s_maxKeepAliveTimerInterval;
}
```

## Related Issues

- OPCFoundation/UA-.NETStandard#4323

## Checklist

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

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [ ] 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.

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

- Fixes #4323

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-30 14:47:50 +00:00
romanett 651beaee1e Fully support AliasNames in ConsoleReferenceServer (#4012) (#4321) 
# Description

The CTT AliasName conformance units skip or fail against the
ConsoleReferenceServer. `AliasName Base` skips with *"No instance of
AliasNameType has been found in the address space"*; `Category Tags`,
`Category Topics` and `Hierarchy` fail. Part 17 support landed in #3779,
but aliases live only inside the `IAliasNameStore` — nothing is
browsable, no `AliasNameCategoryType` is nested under another, and the
Topics aliases point at the Server object rather than at a
`PublishedDataSetType` instance.

Part 17 §6.2 clients — the CTT among them — discover aliases by
browsing, not only by calling `FindAlias`. This PR materializes the
store contents as real nodes and fills the three gaps.

### SDK — `Opc.Ua.Server`

* **`DiagnosticsNodeManager.MaterializeRegisteredAliasNameNodesAsync`**
(new, opt-in) walks every registered `IAliasNameStore` and creates one
`AliasNameType` instance per alias — BrowseName carrying the alias name,
`AliasFor` references to its targets, and the inverse `HasAlias`
reference on each local target (via `externalReferences`, since targets
usually live in another node manager) — plus an `AliasNameCategoryType`
instance for every store category the standard NodeSet does not already
ship. Categories whose NodeId belongs to another node manager are
skipped, so an `AliasNameNodeManager` that also registers its store with
the server-wide registry does not get its NodeIds claimed twice.

Opt-in: servers that only need the well-known `FindAlias` to answer from
their store pay nothing. The nodes are a snapshot taken at address-space
creation — later mutations change what `FindAlias` returns but do not
add or remove nodes.

* **Optional Part 17 methods on the well-known categories.** The same
pass instantiates whatever a category declares through
`AliasNameCapabilities`: `FindAliasVerbose` (§6.3.3),
`AddAliasesToCategory` (§6.3.4), `DeleteAliasesFromCategory` (§6.3.5)
and `LastChange` (§6.3.1). Mutations require a `SecurityAdmin` caller on
a `SignAndEncrypt` channel; the gate previously private to
`AliasNameNodeManager` moved to `AliasNameMethodDispatcher` so both
paths share one implementation.

* **`LastChange`** is now tracked per category rather than only for the
standard `Aliases` node, and the duplicated seeding logic collapsed into
one helper.

### Reference server

* A nested **`Devices`** category under `TagVariables` holds
`Pump1_Status` and `Heater_Power`. `FindAlias` on `TagVariables` still
returns all six names through subtree recursion, while `FindAlias` on
`Devices` returns exactly two — which is what the Hierarchy unit checks.
* `ReferenceServerConfigurationNodeManager` creates three
**`PublishedDataSet` instances** under `PublishedDataSets` (`i=17371`)
and the Topics aliases now target them, satisfying the *"is a Dataset"*
check. These are address-space instances only — no PubSub runtime is
added, and the CTT only inspects the target's type definition.

## Note for reviewers — the reserved NodeIds

The optional children are created at the NodeIds the OPC Foundation
reserves for them (`Aliases.FindAliasVerbose` = `i=24054`,
`TagVariables.LastChange` = `i=32854`, …). Those identifiers are
allocated in `StandardTypes.csv` but are **not** declared in the
ModelDesign and **not** present in the published NodeSet, because the
standard address space does not instantiate optional children. The
source generator therefore emits no parent-to-instance mapping and no
`MethodIds` constants for them, so
`DiagnosticsNodeManager.ReservedChildIds` supplies the nine method ids
and the two missing `LastChange` ids explicitly, each traceable to its
registry row. The 18 argument-property ids do have generated
`VariableIds` constants and are referenced through those.

I deliberately did **not** "fix" this by declaring the children in
`tools/Opc.Ua.SourceGeneration.Core/Design/StandardTypes.xml`: that file
is a verbatim copy of upstream
`UA-ModelCompiler/Design.v105/StandardTypes.xml` (the only differences
today are upstream being newer), so a local edit is lost on the next
sync and the NodeIds would silently revert to factory-minted ones.
`docs/AliasNames.md` records this.

Worth raising upstream separately: `StandardTypes.csv` allocates
`Aliases_FindAliasVerbose` and siblings while the ModelDesign never
declares those children, so the ModelCompiler cannot emit ids it has
itself reserved. If that is ever reconciled, `ReservedChildIds` becomes
redundant and deletes cleanly.

## Related Issues

- Fixes #4012

## 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. — **not
done: verified on net8.0 only** (Server 4710, InformationModel 1099,
Core 4443, plus the client and features alias suites; AOT build clean).
Needs a run against net framework and net10 before this leaves draft.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

### Tests

`tests/Opc.Ua.InformationModel.Tests/Aliasname*Tests.cs` mirror the four
CTT conformance units and no longer `Assert.Ignore` where the CTT
reported failures — the alias suite is 37 tests with no skips. New
`AliasnameOptionalMethodsTests` covers the optional methods, their
reserved NodeIds, the anonymous-caller denial, an add/find/delete round
trip and `LastChange` advancing on mutation.

One pre-existing test changed premise:
`EnumerateSubCategoriesAsyncOnLeafReturnsEmptyAsync` asserted that
`TagVariables` has no sub-categories, which this PR deliberately makes
false. It now targets `Topics` (still a leaf) and a new test guards the
nesting.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-08-29 18:05:42 +00:00
Marc Schier cef83550cb OPC UA Vision: a vision-guided bin-picking demo an LLM agent can see and drive (#4235) 
# Description

Implements **OPC UA — Vision** and uses it to build the demo the
companion specifications were drafted for: an eye-in-hand camera watches
a bin of parts, a language model *looks at the frame*, decides which
part matches the instruction, and commands the robot through Robot
Intent — with the OpenUSD viewer showing the arm move and the bin empty
out.

> **Draft, and stacked.**
[#4195](https://github.com/OPCFoundation/UA-.NETStandard/pull/4195) has
merged and master is merged in, so what is left on top of `master` is
the Vision work plus the unmerged `marcschier/ai-model-management`
branch, which the demo needs for the model/dataset/deployment
provenance. Review the Vision commits; the AI Model Management work
lands with its own PR.

## What is here

**The specification, implemented.** `Opc.Ua.Vision` is generated from
the spec's own `Opc.Ua.Vision.NodeSet2.xml`, vendored byte-identically
(SHA-256 `79C592C6…`, verified against a fresh download and stored
LF-preserved like the Robot Intent NodeSet). `Opc.Ua.Vision.Server`
builds the §4.2 discovery object, sensors, the coordinate-frame tree,
calibrations, media endpoints, inference pipelines and the §9 feedback
methods, with rendering and inference behind injectable providers.
`Opc.Ua.Vision.Client` is built on the generated ObjectType proxies and
composes transforms across the frame tree, which is what turns a
detection in camera coordinates into a pose a robot can act on.

**Two perception paths behind one contract**, which is exactly what
`InferenceLocationEnum` is for. `OnServer` is a deterministic detector
derived from the stage's ground truth — no model, no network, no GPU, so
CI can exercise the whole loop. `EdgeOffServer` is the agent: it sees
the frame over MCP and calls `SubmitDetections`, and the Server
publishes results it did not compute. A client reads
`DetectionResultType` identically either way.

**The agent's eyes.** `tools/Opc.Ua.Mcp.Vision` adds 22 `vision_*`
tools, which with the four shared connection tools makes the `vision`
profile 26; `vision_get_frame` returns the camera image as an MCP
`ImageContentBlock`, so the model looks at pixels rather than reading a
description of them. Tool profiles now compose — `--profile
vision,robotics` yields 62 tools — so one agent can both see and act
without dragging in the full 136-tool catalogue.

**The cell.** `samples/Robotics/BinPickingCell` follows the spec's own
Robotics-Vision Addendum: the `world → robot_base → flange →
gripper_tcp` frame tree with `camera_eih` on the flange, the `HandEye`
extrinsics and the `Intrinsics612x512` intrinsics — the addendum's
2448×2048 calibration scaled to the 4×4-binned grid the camera actually
delivers. `samples/Robotics/BinPickingClient` hosts the composed MCP
catalogue and optionally the viewer.

**Documentation.** `docs/Vision.md` (~800 lines) covers the model,
hosting, the build context, every topology builder, both perception
paths, feedback, the MCP profile and the limitations; linked from
`docs/README.md`, with the profile table in `docs/McpServer.md` and
READMEs for both samples.

## Evidence, not assertions

Each of these was measured on a running system rather than assumed:

| Claim | Evidence |
|---|---|
| The camera sees the bin | 612×512 frame with the bin and all five
parts in view — red `(212,47,47)`, green `(61,193,71)`, blue
`(54,85,218)`, yellow `(215,196,58)`, orange `(209,119,55)` |
| An agent can act on what it sees | Over MCP, `vision_get_frame`
returns a 612×512 PNG that decodes from base64 to 1,254,051 bytes, the
descriptor beside the detections says 612×512, and all five
`BoundingBox2D` centres land **inside** the frame on the pixel colour
their own detection claims |
| The geometry is right | A detection composed `camera_eih → world`
lands on the authored position, residual **0.0000 m** — on both the
on-server and off-server paths |
| The detector tracks the world | Placing a part drops the next scan
from 5 detections to 4, with that part absent |
| The robot obeys the agent | Scripted run took command authority and
completed a pick and a place, both `Succeeded` |
| Tool catalogues are honest | Counted from a running server: vision 26,
robotics 40, `vision,robotics` 62, full 136, **zero duplicates** |
| Every node is browsable by NodeId | Eight representative NodeIds —
both folders, a pipeline, a frame, the sensor, its Calibrations folder,
the HandEye calibration, the Media folder — all `Good` |

That tool-count check matters because the profile tests collapse names
into a `HashSet` and *cannot see* a duplicate registration — so the new
tests count registrations, not names.

## Things that were wrong, and are not now

Writing the tests found three defects of one kind — code that answered
instead of refusing. A zero-norm quaternion was silently rewritten to
identity, so a caller with an all-zero orientation got a confident wrong
grasp pose. `SubmitCorrection` rewrote a null `ResultId` to empty and
called the sink anyway. A bad `StageIdentifier` **killed the host
process** inside native `UsdStage.Open`, via an exception .NET cannot
catch. All three now refuse loudly and are pinned by tests.

**Nodes that existed but could not be reached.** The node manager
indexes the Vision root before configurators run, so everything the
fluent builder grafted on afterwards — both folders, the pipelines with
their Results and Feedback, the frames, the sensors with their optics,
calibrations and media endpoints — was never added to the index.
Browsing *forward from a parent* worked, because that walks
`NodeState.Children` in memory; browsing any of those nodes *by its own
NodeId* returned `BadNodeIdUnknown`, which is how an ordinary client and
the MCP discovery tools navigate. Registration is now deferred and
flushed after each configurator. The first fix covered only the DI path
and left the documented non-DI fallback broken in exactly the same way,
so `ConfigureVisionAsync` now exists and registers what it builds.

That defect survived a green suite because every test browsed children.
The new tests assert by NodeId, and were checked against the broken code
— with the flush removed, they fail.

**Every Method was uncallable.** The Server passed 426 unit tests while
not one of its Methods could be invoked over a real session —
`RunInference` returned `Bad_TooManyArguments`, and so did every other
Method that takes arguments, which is all of them but `StartContinuous`
and `Stop`. Three independent faults, each sufficient on its own: the
Method nodes were never created (they are Optional children, and the
dispatcher guards every attachment with a null check, so it silently
attached nothing); nothing declared the arguments, so the stack
concluded none were expected; and `MethodDeclarationId` pointed at a
synthesised NodeId rather than the NodeSet declaration a client calls
with, so the lookup missed the instance Method entirely. The builder now
creates the Methods, the `Results` folder and the `Feedback` object when
a provider or sink is configured, declares every signature, and names
the right declaration.

Unit tests could not have caught this: they invoke the handler delegates
directly and never go through `MethodState.Call`. It took an end-to-end
test over a real session. Relatedly, children created by the generated
`CreateOrReplace` helpers carry no `ReferenceTypeId`, so nothing could
reach them — fixed once in the build context rather than at the several
dozen call sites.
**And one where I was wrong.** Earlier in this branch I reversed my own
agents' decision and made `SubmitDetections` accept an empty detection
set, reasoning that "I looked and the bin is empty" is a correct
observation and refusing it forces a correct agent to invent a
detection. The reasoning is sound; it is also an argument *against* the
specification rather than an implementation of it. §9.5 is normative and
explicit — its StatusCode table lists `Bad_InvalidArgument` for
"`Detections` empty; or `SubmitCorrection` supplies both or neither
corrected array".

Worse, **none of §9.5's argument rules were enforced anywhere** — the
dispatcher forwarded every submission straight to the feedback sink, so
conformance depended on whichever sink a host installed. They are Server
obligations, so they now live in the dispatcher, ahead of the sink,
proven by strict mocks that would throw if the sink were consulted. The
usability problem is real and does not go away by conforming, so it is
raised where it can be fixed:
[opcua-drafts#70](https://github.com/marcschier/opcua-drafts/issues/70).

## Tracking the updated drafts

The Vision, Robot Intent and AI Model Management NodeSets on
`opcua-drafts` `main` have moved, partly because of feedback this branch
raised. All three are re-vendored byte-identically — verified by
comparing git blob hashes against the upstream files rather than by eye
— and the implementations follow.

**Two statements that were previously inexpressible now have a wire
representation.** `SubmitDetections` takes `SceneIsEmpty` and
`SubmitCorrection` takes `RetractAll`, so an agent can report "I
examined this frame and there is nothing in it" — the terminating
condition of the bin-picking task — and can retract a false positive by
correcting a result down to nothing. Both are checked in both
directions, because the flag is exactly what separates a deliberate
empty observation from a lost payload: an empty array without the flag
is refused, and the flag with an array attached is refused as two
contradictory claims about one frame. Implemented in the dispatcher, the
client, the MCP tools the agent drives, and the sample cell.

Also absorbed: `LampType` and `LightingMode` became enums (§66),
`ProfileName` became `DefaultProfileName` on the endpoint (distinct from
the `ProfileName` a caller passes to `GetStreamEndpoint`, which keeps
its name), AI `Invoke`/`InvokeAsync` gained `PayloadUri` under the same
exactly-one rule, and AI `ListModels` gained a `ContinuationPoint` —
because `MaxResults` alone puts everything past the bound permanently
out of reach.

**OpenUSD is on `0.9.0-alpha`**, verified on the running cell rather
than by restore alone: D3D12 backend, 612×512 frame, the bin and all
five parts in view and detected — the same result `0.8.0-alpha`
produced.

The one local workaround that survives is `VisionMethodArguments`, and
its reason has changed rather than gone away: the builder materialises
these Methods as Optional children through `CreateOrReplace`, which
constructs the state object directly and never runs the generated
factory that now carries the signatures. Checked rather than assumed —
removing the declarations fails five of the eight
`VisionMethodSurfaceTests`.

## Review feedback

Three threads, all addressed in `0fe2803a3`:

- **The AI Model Management work is now a library family, not a
sample.** `src/Opc.Ua.AI` (model), `.Inference` (`IInferenceBackend` and
backends), `.Server` (node manager) and `.Client` (discovery, reads,
calls, artefact transfer), in the same shape as the Robotics and Vision
families; the samples become `samples/AI/ModelManagementServer` and
`ModelManagementClient`. The client library needed real work rather than
relocation: its browsing surface was `private` inside the sample's
scenario runner, so moving the file alone would have produced a library
nobody could call.
- **No cloud-vendor SDK is referenced anywhere.** Inference goes through
`Microsoft.Extensions.AI` — an `IChatClient` is implemented by hosted
services and on-device runtimes alike, which is exactly the property
clause 8.1 asserts. `Azure.Identity` is gone: workload identity is not
an Azure feature, every platform projects a token into a file and each
SDK reads that file behind its own API, so the resolver reads it
directly. The comment claiming `Microsoft.Extensions.AI` would force a
`System.Text.Json` bump was stale — it pins 10.0.10, which this
repository already centralises on.
- **`BinPickingCell` and `BinPickingClient` moved into
`samples/Robotics/`**, which is where they belonged: the cell was
already linking the arm, its kinematics and both USD assets out of
`IntentEnabledRobot`.

## Upstream

Work that belonged elsewhere went elsewhere rather than being worked
around locally:

- **`openusd-dotnet`
[#13](https://github.com/marcschier/openusd-dotnet/pull/13)** — merged
and shipped in `0.8.0-alpha`. `SilkFrameCapture.Capture` rendered only
the first capture of a session and silently returned a blank frame for
every one after it. Also filed
[#14](https://github.com/marcschier/openusd-dotnet/issues/14) (no
camera-by-prim API) and
[#15](https://github.com/marcschier/openusd-dotnet/issues/15) (skipped
tests reported as passed — which fooled me for several runs). Both are
fixed; this branch takes `0.8.0-alpha` and deletes two workarounds,
including 143 lines of hand-rolled projection maths.
- **`opcua-drafts`
[#66](https://github.com/marcschier/opcua-drafts/issues/66)–[#69](https://github.com/marcschier/opcua-drafts/issues/69)**
— a close read of the spec produced 22 candidate ambiguities; 12 were
discarded as misreadings with clause citations and the remaining 10
filed. One is concrete: five members exist in the NodeId CSV that no
clause documents.
- **`opcua-drafts`
[#70](https://github.com/marcschier/opcua-drafts/issues/70)** — §9.5
could not express an empty observation or a false-positive retraction.
**Adopted**, with the explicit-flag resolution argued for:
`SubmitDetections` gained `SceneIsEmpty`, `SubmitCorrection` gained
`RetractAll`, and the corrected-array rule relaxed from *exactly one* to
*at most one*.
- **`opcua-drafts`
[#71](https://github.com/marcschier/opcua-drafts/issues/71)** — the
Vision NodeSet browse-named all 13 `InputArguments` / `OutputArguments`
Properties in the Vision namespace rather than the base namespace, so a
stack could not find them and every Method in the specification was
uncallable. **Fixed**; this branch now tracks the corrected artefact.

## The defect underneath it all

The end-to-end suite was removed earlier in this branch because nine of
its
fourteen tests failed on one symptom: a client could not open a
pipeline's
`Feedback` object or enumerate its `Results`, even though browsing the
same
parent listed both. Restoring it meant explaining that, and the
explanation
was not in Vision.

`RelativePathElement` initialised **`IsInverse` to `true`**. A caller
that
sets a `ReferenceTypeId` and a `TargetName` — which reads as *"the child
reached by this Reference"* — was therefore asking the Server for the
**inverse** Reference, and got `Bad_NoMatch` with nothing in the request
to
suggest why.

`ObjectTypeClient.ResolveChildNodeIdAsync` is such a caller, and it is
what
**every source-generated Optional-child accessor in the stack** is built
on
— the accessor whose own documentation cites
`AlarmConditionTypeClient.GetShelvingStateAsync`
as an example. Vision is merely where it surfaced, because Vision puts
two
Optional children on the path an off-Server agent must take. The sibling
helper in `StateMachineTypeClientExtensions` already set `IsInverse =
false`
by hand, which is the shape of a trap rather than a default.

The default is now forward, `ResolveChildNodeIdAsync` says so
explicitly,
and the existing test that had recorded `true` as *expected* now pins
`false` with the reasoning. Of the forty `RelativePathElement`
construction
sites in the tree, that accessor was **the only one relying on the
default at
all** — every deliberate caller already set it.

Two further defects were found underneath it, both real and both fixed:

- The generated `CreateOrReplace` helpers construct a child state object
directly and leave **`TypeDefinitionId`** unset as well as
`ReferenceTypeId`.
A child referenced by nothing cannot be browsed; one with no type
definition
is a malformed Object that a client filtering by type silently skips —
which
  is exactly what `EnumerateResultsAsync` does. Normalisation moved to
`VisionNodeManager`, so it also covers the case the builder cannot see:
a
result published **at runtime** by an inference provider, long after the
address space was built. That path produced results a client could list
but
  not read.
- `Opc.Ua.AI` shipped without an `AssemblyInfo`, so the solution built
with
  six `CA1014` warnings.

I had initially "fixed" this in the Vision client with Browse-based
fallbacks,
on the strength of a probe that appeared to show
`TranslateBrowsePathsToNodeIds`
failing even for `Server → ServerStatus`. That probe was wrong: it used
an
unqualified `ReferenceTypeIds`, which inside a `Opc.Ua.Vision.*`
namespace
binds to *Vision's* `ReferenceTypeIds` and yields a nonsense NodeId.
Chasing
that to the end is what turned up the real cause. **The fallbacks are
gone** —
they would have hidden a stack-wide bug behind a Vision-shaped
workaround, and
one of them matched on browse name while ignoring the namespace index.

`tests/Opc.Ua.Vision.Intent.Tests` is restored and back in `UA.slnx`,
passing
**14/14** on its own merit rather than through a client-side workaround.
A
registration test now pins the pipeline's `Feedback` and `Results`
children,
which nothing covered before.

## Running it, and what that found

The demo's premise is that a language model **looks at the bin and
reasons
about what it sees**. Running it end to end showed it could not, for
reasons
no test covered, because every one of them lived between components.

**The sample did not compile.** Two `CS8604` nullable errors with
`TreatWarningsAsErrors`, so neither the cell server nor the demo could
be
started at all. Worth recording that my first explanation was wrong: CI
*does*
build this project - the Linux leg's `net10.0` pass compiles it,
log-confirmed -
so this is not a coverage gap in the workflow, and no workflow change
was made.

**`Pick` and `Place` never moved the arm.** They were a delay plus a
gripper
action, so the operations reported `Succeeded` while nothing travelled.
They now
solve IK once and interpolate in joint space. **And the parts never
moved**: the
arm swung but the bin stayed full, because nothing tied the held object
to the
world state. The connector now creates 11 monitored items where it
created 6,
and a scripted run delivers `SeqNo=151` across 10 messages where it
delivered
`SeqNo=3` across 1.

**The frame was smuggled through a string field.** The media provider
put a
base64 *data URI* of the PNG into `VisionImageReferenceDataType.Uri` - a
field
meant to be a *reference*, while the bytes were already returned in the
inline
`ByteString`. Every response therefore shipped the image twice, and a
1,311,400-byte PNG became a 1,748,536-character string against a
`MaxStringLength` of 65,535: **26.7x over**. That is why raising
`MaxByteStringLength`, `MaxArrayLength` and `MaxMessageSize` never
helped - none
of them govern a `String`. Separately the MCP tool assigned raw bytes
where
`ImageContentBlock` requires base64, so the wire carried
`"data":"�PNG..."` -
every non-UTF-8 byte replaced by U+FFFD, destroying the image and
inflating the
response to 6.8 MB.

**The camera was aimed at nothing.** The arm's home pose, the flange
scan pose
the Vision model declared and the joint angles in the USD stage were
three
independent claims about where the camera was, and they disagreed - the
declared
flange orientation pointed the view 86 degrees away from the projection
camera.
Solving them as one thing exposed two further constraints that are easy
to miss:
the camera has to sit off the tool axis or it photographs its own
gripper, and
the IK branch has to be elbow-back or a link parks under the camera and
fills the
frame with the arm's own upper arm. A third was a regression this branch
introduced and `Opc.Ua.Robotics.Tests` caught - aiming a straight-down
camera
from a point on the base's own X-Z plane puts the wrist on the J4/J6
singularity, so every motion away from home fails to solve. A 15-degree
camera
roll, free because the camera still looks straight down, gets 25 degrees
clear,
which is the margin the poses before this branch had.

**The frame and the detections were different images.** The sensor
declared
2448x2048, the clip endpoint 1280x1024, and the renderer produced
640x512 - not
even the same aspect ratio. The ground-truth detector projects through
the
declared intrinsics, so its boxes lived in 2448x2048 space while an
agent was
handed a 640x512 picture: *"pick the red cube you can see"* arrived with
coordinates that pointed off the image. Everything now agrees at
612x512, an
exact 4x4 bin of the native sensor with the intrinsics scaled to match;
the
native size stays in the model and serial number, where it describes the
hardware rather than the image.

**`LatestClip` was created and never written**, so it reported
`Bad_NoDataAvailable` for the life of the Server and a consumer
following the
model - read the published frame, call the method only if there is none
- never
got a frame. The dispatcher now publishes a clip it has just encoded,
for every
Vision server rather than this sample alone.

One genuine API gap turned up on the way: `OpcUaServerOptions` exposes
`MaxByteStringLength`, `MaxArrayLength` and `MaxMessageSize` and **no
`MaxStringLength`**, so a Server that legitimately needs a long string
could not
raise the limit through the hosting API at all. Added, with tests. It is
not the
fix for anything above - that was a design bug - but the missing knob
was real.

## Not finished

- **Two projects sit below the repository's 80% line coverage.**
`Opc.Ua.Vision.OpenUsd` is at 71.1% — what is left only executes with a
real OpenUSD stage and plugin tree, which is not on every build agent.
`Opc.Ua.AI.Inference` is now at **86.7%**, up from 12.9%: the code this
branch *wrote* is covered by 25 tests, and `RestChatCompletionsBackend`
— previously the whole of the shortfall — is covered by 18 more against
a stubbed `HttpMessageHandler`, exercising success, refusal, throttling,
retry-after, timeout and cancellation without a network. Everything else
clears the bar — `Opc.Ua.Vision.Server` 92.1%, `Opc.Ua.AI` 91.4%,
`.Vision.Client` 88.1%, `Opc.Ua.Vision` 85.7%, `Opc.Ua.AI.Server` 80.1%,
all measured with coverlet. The CI coverage gate reports itself as
advisory and non-blocking.
- **`SamplesCollected` is not counted here, and `docs/Vision.md` now
says so.** Section 9.4 requires a Server to count a negative example
(`SceneIsEmpty` / `RetractAll` carrying a `GroundTruthLabel`) exactly as
it counts one carrying geometry. `VisionNodeManager` does not, and
structurally cannot: `SamplesCollected` is a property of
`LearningJobType`, which the *AI Model Management* companion defines,
and Vision reaches the job through a `NodeId` value rather than a
Reference **precisely so this model takes no dependency on the model
that defines it**. What the Server does guarantee is that the negative
example survives the hop intact — both flags are carried verbatim to
`IVisionFeedbackSink` — so a host that binds a learning job has
everything it needs to satisfy the clause on the counter it owns. Stated
in the limitations rather than left implied.

## Related Issues

Was stacked on #4195, now merged. No tracking issue yet for the Vision
implementation itself — happy to open one if you would like the design
recorded as an ADR before this goes further.

## 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. — *Types 8562, Server 4072,
Client 2123, InformationModel 1063, Robotics 540, Tools 466, Vision 442,
AI 74 and the restored Vision+Intent end-to-end suite 14/14, all green
on net10.0. Only `.OpenUsd` (71.1%) is below the 80% bar — see Not
finished.*
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings. — *`dotnet build UA.slnx -c Release`: 0 errors, 0
warnings.*
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. — *net48:
Types 8555, Client 2126, Vision 385 green. Two net48 Server tests fail
(`CreateUserManagementSeamBindsTheModelToTheServerAsync`, and
`DurableDataValueQueueVerifyReferenceBatchingAsync` only under parallel
load); both were confirmed **pre-existing** by re-running them with the
`IsInverse` change reverted, which fails identically.*
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings. — *The net48 build legs failed because the
restored end-to-end project used `ValueTask.FromResult` /
`string.Create`, which do not exist on .NET Framework; fixed in
`fb57522da` and verified building on every TFM.
`DisposeDrainsHeldConnectionCallbackBeforeListenerDisposal` failed once
on the Ubuntu Client leg — a 13 ms timing-sensitive reverse-connect
disposal test that passes locally 2123/2123 and touches no browse
paths.*
- [ ] I have addressed **all** PR feedback received.
## Typed Vision-guided Robotics MCP workflows

The branch now also makes the demo workflow first-class in the Full MCP
profile:

- replaces stringified Robotics intent and mission inputs with typed
request schemas and exact name-or-NodeId selectors;
- adds bounded operation/mission paging, stable step-to-operation
identity, retained mission discovery, and `robotics_wait_mission`;
- makes `vision_run_inference` return bounded typed detection,
inspection, or segmentation summaries in one call;
- adds same-session `robotics_vision_pick` to select a detection and
submit Pick or Pick/Place work without implicitly taking authority,
retrying, cancelling, or waiting; and
- starts Publish workers for classic subscriptions so an independently
launched OpenUSD viewer receives live robot and part updates.

The migration guide documents the intentionally breaking MCP
request-schema changes.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
Copilot-Session: 99d1ab42-f83a-41ab-bf3e-85bf81fadace
Copilot-Session: bcbc8ac9-5abd-42a8-8bac-e10533790e2e
 2026-08-29 19:34:15 +02:00
romanett 70c8ce4649 Allow passing an existing config XML to the client DI AddClient (#4328) 
# Description

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

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

New API surface:

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

Mechanism and behavior:

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

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

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

## Related Issues

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

## Checklist

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

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

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-08-29 13:33:40 +00:00
romanett e61ad4b306 Fully support sub-state machines in the fluent state-machine builder (#4322) 
# Description

Part 16 §B.3 places `HasSubStateMachine` on the parent **state** node,
but the fluent builder attached it to the state-machine **root** because
`FluentFiniteStateMachineState` never materialized state nodes at all.
Investigating #3821 showed the gap was wider than the reference
placement:

1. **No state nodes existed.** The NodeId written into `CurrentState/Id`
— `new NodeId(stateId, ElementNamespaceIndex)` — was **dangling**: no
node with that id was in the address space.
2. **`AvailableStates` was never populated**, so
`GetAvailableStatesAsync` returned empty against a fluent-built server.
3. **The whole client discovery chain was therefore dead.**
`ObserveEffectiveStateAsync` enumerates `AvailableStates` and browses
`HasSubStateMachine` from each state node — it found nothing. The client
side was already spec-correct; only the server deviated.
4. **`HasSubStateMachine` is non-hierarchical** (a subtype of
`NonHierarchicalReferences`, `i=32`). Using it as the sub-SM's
`BaseInstanceState.ReferenceTypeId` left the sub-SM unreachable from a
hierarchical browse of the machine.

## What changed

**Element nodes are materialized.** Definition-mode machines now create
one `StateType` node per declared state and one `TransitionType` node
per declared transition — each a `HasComponent` child carrying
`StateNumber` / `TransitionNumber`, with the §B.4 `FromState` /
`ToState` / `HasEffect` references and `HasCause` added by `WithCause`.
`AvailableStates`, `AvailableTransitions` and `LastTransition` are
populated, so `CurrentState/Id` and `LastTransition/Id` resolve to real,
browsable nodes. NodeIds are per-instance, so several machines built
from one definition coexist.

**`HasSubStateMachine` moved to the state node**, with the inverse
`SubStateMachineOf`, and the sub-SM is attached as a `HasComponent`
child (fixing 4).

**Core gains element-id mapping hooks.** `GetCurrentStateId` /
`UpdateStateVariable` / `UpdateTransitionVariable` hard-coded the
numeric form and were non-virtual, so a subclass could not redirect
them. They now route through `public virtual` `GetStateNodeId` /
`GetStateId` / `GetTransitionNodeId` / `GetTransitionId`, whose defaults
reproduce the previous `(elementId, ElementNamespaceIndex)` convention
**exactly** — every stack-shipped and generator-emitted machine is
unaffected, and there is a regression test asserting that.

**Generated machines report their model namespace.** The generator now
emits an `ElementNamespaceUri` override on `FiniteStateMachineType`
subtypes, taken from the nearest type that actually *declares* state
children — a subtype that only adds behaviour inherits its elements, and
their namespace, from its base.

**Ordering and validation fixes.** `WithSubStateMachine` and
`WithInitialState` are now order-independent (previously, attaching a
sub-SM before setting the initial state left the child suspended until
the parent left and re-entered the state — silently).
`WithSubStateMachine` validates `parentStateId`, and the builder rejects
duplicate element browse names, since element NodeIds are now derived
from them.

## A defect this surfaced

Code that assembles nodes with the generated `CreateInstanceOf<Type>`
factories never calls `NodeState.CreateAsPredefinedNode`, so
`OnBeforeCreate` / `OnAfterCreate` are skipped for the entire subtree —
and `OnAfterCreate` is the only place a state machine resolves its
element namespace. It is also where `ConditionState` binds its `Enable`
/ `Disable` / `AddComment` handlers and `AlarmConditionState` binds the
shelving methods, so the impact is broader than state machines.

This PR fixes only the two call sites it depends on — DI's
`SoftwareUpdateFacetWiring` and Robotics' `RoboticsOperationBuilder`.
**The repository-wide audit is deliberately left out of scope and
tracked in #4320** (24 files call these factories; none completed the
lifecycle).

The Robotics regression had no test coverage at all, so an assertion on
`CurrentState/Id` was added and verified to fail without the fix.

## Breaking changes

- Sub-state machines are **no longer browsable from the state-machine
root**. Callers passing a machine's `ObjectId` to
`GetSubStateMachineAsync` must pass a state NodeId instead — from
`GetAvailableStatesAsync` or a snapshot's `CurrentStateId`.
- `CurrentState/Id` for fluent machines changes from the numeric form to
the materialized node's id. Code reading a numeric id out of it should
call `FiniteStateMachineState.GetStateId` rather than parsing the
identifier.
- Generated machines qualify `CurrentState/Id` / `LastTransition/Id`
with their companion-spec namespace instead of the OPC UA one.

Documented in `docs/WhatsNewIn2.0.md` and `docs/StateMachines.md`.

## Notes for reviewers

- Adding `virtual` to the `FiniteStateMachineState` members is a binary
(not source) breaking change — assumed acceptable on the 2.0 line, but
worth confirming against the compatibility policy.
- Address-space cost is one node plus one property per declared state
and transition, per machine instance; called out in the docs for servers
instantiating many machines.
- `AvailableTransitions` is modelled as a `HasComponent`
`BaseDataVariable` (matching the standard NodeSet and what the client's
browse path expects), not a Property.

## Related Issues

- Fixes #3821
- Surfaced #4320 (node create lifecycle audit — not addressed here)

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, 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.

### Test runs (net10.0)

| Suite | Result |
|---|---|
| `Opc.Ua.Server.Tests` (full) | 4734 passed, 5 skipped |
| `Opc.Ua.SourceGeneration.Core.Tests` | 3780 passed |
| `Opc.Ua.SourceGeneration.Tests` | 132 passed |
| `Opc.Ua.SourceGeneration.Stack.Tests` | 94 passed |
| `Opc.Ua.Types.Tests` | 8763 passed |
| `Opc.Ua.Robotics.Tests` | 540 passed |
| `Opc.Ua.Robotics.Intent.Tests` | 22 passed |
| `Opc.Ua.Di.Tests` | 424 passed |
| `Opc.Ua.ISA95.Tests` | 136 passed |
| `Opc.Ua.Aas.Tests` | 670 passed |
| `Opc.Ua.Positioning.Tests` | 69 passed |
| `Opc.Ua.Client.Tests` (StateMachines) | 36 passed |

Only net10.0 was run locally; the .NET Framework leg is left to CI,
hence the unchecked box above.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-08-29 13:33:16 +00:00
romanett f4e93eab93 Fix public IsNodeInView/IsNodeInViewAsync overloads always returning false (#4333) 
# Description

The public `IsNodeInView(OperationContext, NodeId, object)` in
`CustomNodeManager2` and `IsNodeInViewAsync(OperationContext, NodeId,
object, CancellationToken)` in `AsyncCustomNodeManager` unwrap the
`NodeHandle` and forward `handle.Node` together with the **unchanged**
`OperationContext`. Since there is no `NodeState`-taking overload with
an `OperationContext`, overload resolution binds the call back to the
same public `(object)` overload; the recursive call's `is not
NodeHandle` guard then returns false. As a result both entry points
always reported "not in view" for a validated handle, regardless of the
view. The sync variant has behaved this way since the initial commit;
the async port copied the same pattern.

The defect is latent in the SDK today: nothing calls
`INodeManager2.IsNodeInView` / `IsNodeInViewAsync` except the sync/async
adapters forwarding to each other, and Browse view filtering flows
through the protected virtuals directly. External code calling the
public API (e.g. node managers ported from the samples) was affected.

Changes:

- Both public overloads now copy the operation context
(`SystemContext.Copy(context)`) and delegate to the overridable
protected `IsNodeInView(ServerSystemContext, NodeId, NodeState)` helper
— the same virtual Browse filtering uses and sub-classes override. Doc
remarks name the virtual to override. `IsNodeInViewAsync` drops its
now-unneeded `async` keyword (signature unchanged).
- New regression test
`IsNodeInViewAsync_HandleWithNodeUsesOverridableIsNodeInViewAsync` runs
against the native async manager (both monitored-item manager
configurations) and against `CustomNodeManager2` behind
`AsyncNodeManagerAdapter`. It covers: known view → true, unknown view →
false, placeholder handle without node → false, the sync entry point via
`SyncNodeManager`, and — via an override hook on the testable managers —
that the public entry points genuinely route through the overridable
protected virtual. The test was verified to fail against the unfixed
code.
- The characterization test that locked in the broken sync behaviour
(`IsNodeInView_ValidatedHandleReturnsFalseFromPublicOverloadRecursion`,
flagged "not fixed here") is replaced by
`IsNodeInView_ValidatedHandleDelegatesToProtectedOverload`, asserting
the fixed behaviour.

Verified locally: all `IsNodeInView*` tests and the full node-manager
suites (484 tests) pass on net8.0; `Opc.Ua.Server` builds warning-free
on net472.

## Related Issues

Found while migrating the Views sample, whose view filtering uses the
protected sync virtuals and is unaffected.

- OPCFoundation/UA-.NETStandard-Samples#758

## Checklist

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

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

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

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-29 13:59:50 +02:00
Copilot 33ed3028e6 Fix macOS test host crash caused by indefinite keepAlive worker await in session teardown (#4319) 
`test-macOS-latest-History` CI job crashes after all 293 tests pass. All
tests complete successfully but the test host exits with code 1 ("Test
host process crashed") ~35 seconds into teardown.

**Root cause chain:**
- `Session.StopKeepAliveTimerAsync` unconditionally `await`s
`keepAliveWorker`, which is blocked in `ReadAsync`. On macOS under CI
load, socket reads do not honor `CancellationToken`, causing session
teardown to hang indefinitely.
- After 15 s, `TestFixture.OneTimeTearDown`'s watchdog fires, sets
`s_skipRemainingTeardowns = true`, and bails out—without calling
`ServerFixture.StopAsync()`, leaving certificate managers un-disposed.
- Certificate leak assertions fire in `GlobalTeardown`, triggering
`ArmProcessExitWatchdog` → `Environment.Exit(0)` on a background thread
severs the VSTest pipe → "Test host process crashed", exit code 1.

## Changes

**`src/Opc.Ua.Client/Session/Session.cs`**
- `StopKeepAliveTimerAsync`: replace unconditional `await
keepAliveWorker` with a 5-second bounded `Task.WhenAny`. If the worker
doesn't stop in time, log a warning and abandon it. Well-behaved
transports cancel immediately; the bound guards against unresponsive
macOS sockets.
- Added `s_keepAliveStopTimeout` static field (5 s) and
`KeepAliveWorkerDidNotStopWithinTimeout` source-generated log message
(`Session+69`).

```csharp
// Before
await keepAliveWorker.ConfigureAwait(false);

// After
var completed = await Task.WhenAny(keepAliveWorker,
    Task.Delay(s_keepAliveStopTimeout)).ConfigureAwait(false);
if (completed != keepAliveWorker)
{
    SessionLog.KeepAliveWorkerDidNotStopWithinTimeout(m_logger, s_keepAliveStopTimeout);
}
```

**`tests/Opc.Ua.Client.TestFramework/TestFixture.cs`**
- `OneTimeTearDown`: both the early-skip and post-session-close skip
paths now call `ServerFixture.StopAsync()` before nulling the reference.
`StopAsync` fast-paths when `s_skipRemainingTeardowns` is set—disposes
certificate managers without blocking on server shutdown—breaking the
leak→crash cascade.

## Related Issues

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] 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-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>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-29 10:34:55 +00:00
romanett b9657e5f0d Allow passing an existing config XML to the hosted server DI AddServer (#4325) 
# Description

Extends the hosted-server dependency-injection surface so applications
that already own a classic `ApplicationConfiguration` XML file
(`*.Config.xml`) have a direct migration path to DI with **all** of
their existing settings applied.

New API surface:

- `AddServer(string configurationFile, Action<ApplicationConfiguration>?
configure = null)` and the `AddServer<TServer>` variant load the
configuration from the file instead of building it from
`OpcUaServerOptions`.
- `AddServer(Stream configurationStream, ...)` (plus generic variant)
for documents that are not files on disk, e.g. embedded resources. The
stream is read once and disposed by the hosted service during startup.
- `OpcUaServerOptions.ConfigurationFile` / `ConfigurationStream` back
the overloads, so the options-callback route works too and the file path
can be bound from configuration (`OpcUa:Server:ConfigurationFile`).
- `OpcUaServerOptions.ConfigureLoadedConfiguration` runs after the
document is loaded and validated but before certificates are checked, so
individual settings can be overridden from code without editing the
file.

Behavior:

- The document is loaded through
`ApplicationInstance.LoadApplicationConfigurationAsync`, so every
setting (base addresses, security policies, certificate stores,
transport quotas, operation limits, user token policies) applies exactly
as on the classic path, including localhost fix-up.
- The supplied document is authoritative: the builder-feeding options
(`ApplicationName`, `EndpointUrls`, `PkiRoot`, policy toggles,
`ConfigureBuilder`, ...) are not applied on this path, and it takes
precedence over a shared application registered with
`ConfigureApplication(...)`. Options acting on the hosted server itself
(`Identity`, `ConfigureRateLimits`) and all fluent registrations
(`AddNodeManager`, `ConfigureRoles`, identity authenticators, ...)
compose unchanged.
- DI-registered `ICertificateManager` / `ICertificatePasswordProvider`
are honored, the "listening at" log reports the loaded base addresses,
and the unmatched user-token-policy startup warning is derived from the
policies the document advertises.
- Setting both `ConfigurationFile` and `ConfigurationStream` fails
startup with a clear `InvalidOperationException`; a missing file fails
startup with the original `ServiceResultException`.

Coverage: new `ConfigurationFileHostingTests` fixture (12 tests) with
end-to-end hosted starts from file and stream asserting the loaded
settings, custom `TServer` types, the override callback, precedence over
`ConfigureApplication`, warning derivation from the document,
missing-file and file+stream startup failures, argument validation, the
duplicate-registration guard, and configuration-section binding.

Docs: new "Migrating with an existing configuration XML file" section in
`docs/DependencyInjection.md` (plus first-class options table rows), a
pointer in the 2.0 migration guide, and a NuGet README mention.

## Related Issues

None — additive convenience overloads on the existing hosted-server
dependency-injection surface; no behavior change for existing
registrations.

## Checklist

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

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

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-29 10:48:24 +02:00
romanett 41db5aca60 Fluent: publish Configure-created nodes' references on foreign-manager nodes (Objects folder placement) (#4331) 
# Description

A node created inside the fluent `Configure` hook (source-generated
`[NodeManager]` partial, or the hosting `AddNodeManager(uri, build)`
route) had no way to appear below the Objects folder — or below any node
owned by another node manager.
`AsyncCustomNodeManager.AddReverseReferencesAsync` mirrors inverse
references into the `externalReferences` dictionary **before** the
`Configure` partials run, and the fluent surface never sees the
dictionary, so a configure-created node with an inverse `Organizes`
reference to `ObjectsFolder` existed and simulated but never showed up
under Objects.

This implements the three coordinated pieces proposed in #4329, plus the
companion namespace gap:

**1. Idempotent mirroring pass** (independently worthwhile hardening)
- `AddExternalReference` in `AsyncCustomNodeManager` and
`CustomNodeManager2` now has if-missing semantics (linear scan of the
per-source list).
- `TypeTable.AddEncoding` no longer appends a duplicate encoding id;
re-registering the same encoding is a succeeding no-op.
(`AddRootNotifierAsync` and the node-side `AddReferenceIfMissing` branch
were already idempotent.)

**2. Re-run the pass after `Configure`**
- New protected
`FluentNodeManagerBase.CompleteConfigureAsync(externalReferences, ct)`
wraps `AddReverseReferencesAsync` so hand-written managers using
`CreateFluentBuilder` benefit too.
- The generator-emitted `CreateAddressSpaceAsync` and the hosting
`FluentNodeManager` invoke it once between the `Configure` callbacks and
`Seal()`. Timing is safe: the master distributes `externalReferences`
only after every manager's `CreateAddressSpaceAsync` returns. Side
benefit: inverse `HasNotifier` references on configure-created event
sources get root-notifier registration for free.
- The hosting `FluentNodeManager`'s hand-rolled `externalReferences`
workaround for its root folder is deleted — the root's inverse
`Organizes` reference now flows through the shared pass (mirrored
exactly once, covered by a test).

**3. Discoverable fluent sugar**
- `OrganizedBy(parentId)` / `UnderObjectsFolder()` on `INodeBuilder`
write the inverse `Organizes` reference; with piece 2, placement is just
"write the inverse reference".
- A parentless `CreateInstance<TState>` on `INodeManagerBuilder` takes a
constructor-style factory (`p => new BoilerState(p)`, keeping the
surface reflection-free and AOT-safe), materializes the subtree from the
type model via `NodeState.Create`, rebases all NodeIds through the
manager's `INodeIdFactory`
(`AssignInstanceNodeId`/`AssignInstanceChildNodeIds` — the same pair the
generated factories use, so declaration-id children cannot collide with
the type model), and registers via `AddPredefinedNodeSynchronously`. The
Boiler #2 shape becomes fully fluent:

```csharp
builder.CreateInstance(
        new QualifiedName("Boiler #2", NamespaceIndexes[1]),
        p => new BoilerState(p))
    .Configure(n => n.UnderObjectsFolder());
```

**Companion gap, same theme:** `[NodeManager]` grows
`AdditionalNamespaceUris`, flowing through attribute discovery → binding
→ generator so the generated constructor reports a second (instance)
namespace at construction and the generated factory advertises it in
`NamespacesUris` — today `SetNamespaces` after construction is not
enough because `MasterNodeManager` builds its namespace routing from
what the manager reported when it was built.

**Scope boundary** (as discussed in the issue): startup-time
configuration only; for nodes created after startup the correct
primitive remains `IMasterNodeManager.AddReferencesAsync`.

**Reviewer notes**
- The generated-code change was validated against the real pipeline: a
forced fresh compile of `Opc.Ua.Server.Tests` (whose CoverageNodeSet
managers are `[NodeManager]`-generated) emits `await
CompleteConfigureAsync(externalReferences, cancellationToken)` between
`Configure` and `Seal()` and compiles against the real base class.
- The distribution consumer (`AddReferencesAsync`) already used
`AddReferenceIfMissing`, so the whole chain is defense-in-depth
idempotent.
- 15 new tests: double-run idempotence, second-pass pickup of
late-registered nodes, the new reference sugar, root-instance creation
incl. child-id rebasing and reference remapping, hosting end-to-end
Objects-folder placement, emitted-sequence and `AdditionalNamespaceUris`
generator tests, and a full Roslyn-pipeline attribute test. Docs updated
in `docs/NodeManagers.md` (new "Creating nodes under other managers'
nodes" section, attribute docs, fluent-surface sections).
- Verified locally: all touched projects build with 0 warnings; full
`Opc.Ua.Server.Tests` (4,725 passed / 0 failed / 5 skipped, net9.0),
`TypeTableTests` (113), `Opc.Ua.SourceGeneration.Core.Tests` (41),
`Opc.Ua.SourceGeneration.Tests` (132), and the Quickstarts.Servers
sample all pass.

## Related Issues

- Fixes #4329
- Motivating case: OPCFoundation/UA-.NETStandard-Samples#756 /
OPCFoundation/UA-.NETStandard-Samples#795 (Workshop/Boiler migration)

## 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 Fable 5 <noreply@anthropic.com>
 2026-08-29 10:38:56 +02:00
romanett 98c5438395 Bind monitored items to their owning node manager instead of per-call adapter wrappers (#4336) 
# Description

`AsyncCustomNodeManager` passed `m_syncNodeManager.ToAsyncNodeManager()`
as the owning node manager when creating and restoring monitored items.
Since `SyncNodeManagerAdapter` does not implement `IAsyncNodeManager`,
every call allocated a fresh `AsyncNodeManagerAdapter` around the
`SyncNodeManagerAdapter` around the manager itself. The double-wrapped
chain:

- routed async dispatch through the sync adapter's
`GetAwaiter().GetResult()` blocking (sync-over-async), defeating the
purpose of the async node manager,
- hid `INodeManagerMonitoredItemLifecycle` — the inner sync adapter does
not implement it, so detach during monitored-item recovery degraded to
`BadNotSupported` and ownership snapshots came back empty,
- broke reference-identity checks against the owning node manager and
gave every item a distinct `NodeManager` instance.

The manager implements `IAsyncNodeManager` natively, so it now passes
`this` at both call sites — the same reference its constructor already
hands to the monitored item managers.

The second commit applies the milder counterpart to the sync
`CustomNodeManager2`, which called `this.ToAsyncNodeManager()` at five
sites and thereby allocated a fresh single-wrap adapter per created
monitored item. The adapter is now created once in the constructor and
reused, so the monitored item managers, created and restored items, and
detached-item recovery all share one `NodeManager` reference instead of
relying on the `SyncNodeManager` fallbacks in
`AreSameNodeManager`/`GetVisibleNodeManager`.

The regression test
`CreateMonitoredItemsAsyncBindsOwningNodeManagerAsync` runs in all three
fixture variants (MonitoredNode, SamplingGroup, CustomNodeManager2 via
adapter) and asserts the created items bind to the manager itself, share
one `NodeManager` reference, and expose a working monitored-item
lifecycle snapshot. Both halves were verified to fail without their
respective fix.

## Related Issues

- Fixes #4334

## Checklist

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

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

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-29 10:31:31 +02:00
romanett a504c3e248 Fix certificate leak when rejected-store enqueue races manager disposal (#4337) 
The macOS legs of the Build and Test workflow intermittently failed the
assembly-level teardown of the Sessions (also History and Client) test
suites with "Certificate leak detected: 1 instance(s) created but not
disposed" while every test passed (master run 33004556144:
created=16777, disposed=16776; PR run 33207357247: created=17373,
disposed=17372). The count held steady through the teardown's full
quiescence window, so the missing disposal was permanent, not a slow
in-flight drain.

RejectedCertificateProcessor.EnqueueAsync takes a per-certificate AddRef
for the queued WriteRequest before TryWrite. Once
CertificateManager.DisposeAsync has completed the channel writer, the
bounded DropOldest channel refuses TryWrite and the WriteAsync fallback
throws ChannelClosedException, abandoning the request: nothing releases
the references it owns, so every certificate in the chain keeps one
leaked reference for the life of the process. A validation that races
manager disposal — a keep-alive, reconnect or fire-and-forget channel
teardown rejecting a certificate while the fixture tears down — lands in
exactly this window, and the slower loaded macOS runners make that race
likely. The abandoned request also left FlushRejectedAsync waiters
hanging on a drain marker that never resolves, and surfaced unobserved
ChannelClosedExceptions through the fire-and-forget
EnqueueRejectedCertificate discard.

Route both enqueue paths through a fallback that, when the write cannot
commit the request to the channel, disposes the request's chain
(releasing the per-certificate references), resolves its completion as
"not processed" so drain waiters are unblocked, and swallows only the
benign ChannelClosedException shutdown race; cancellation still
propagates. The teardown leak detector needs no change: its quiescence
poll was correctly reporting a genuine leak.

The new regression test reproduces the CI signature deterministically
against the previous code (ChannelClosedException plus "Certificate leak
detected: 1 instance(s) created but not disposed (created=1,
disposed=0)" in the assembly teardown) and passes with the fix.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-29 10:30:19 +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
Copilot 3f52d5b425 Wait for HTTPS listener readiness in integration tests (#4313) 
# Description

The real HTTPS listener test could issue its first request before
Kestrel accepted connections, causing intermittent macOS failures.

- **Listener readiness**
  - Probe the loopback port after `OpenAsync`.
  - Retry connection failures every 25 ms with a 10-second deadline.
- Create the test client only after the listener accepts TCP
connections.

## Related Issues

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] 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.

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

- Fixes #4307

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: romanett <romanett98@gmail.com>
 2026-08-26 16:39:57 -07: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
Marc Schier b292e243c6 Add trusted NuGet release workflows (#4312) 
## Summary

- replace the preview-only workflow with `nuget-publish.yml`, running on
every branch push, `v*` tag, and manual dispatch
- build and sign all Release packages plus genuine `.Debug` packages,
publish normal packages to GitHub Packages, and retain exact signed
package and symbol bytes
- add manual `release.yml` promotion from a successful `release/*`
publish run to nuget.org through `NuGet/login@v1` trusted publishing
- validate package identities, versions, signatures, symbols,
duplicates, and SHA-256 hashes before publication and promotion

## Signing configuration

All required repository Actions secrets are configured, including
`OPCFOUNDATION_NETSTANDARD_SNK_BASE64`, which was transferred directly
from Azure DevOps Secure Files without placing the private key in a
pipeline artifact.

## nuget.org setup

The account-side trusted publishing policy must use:

- Repository Owner: `OPCFoundation`
- Repository: `UA-.NETStandard`
- Workflow File: `release.yml`
- Environment: `release`

The existing `release` environment contains `NUGET_USER` and is
restricted to `release/*` branches.

## Validation

- parsed both workflow YAML files against the GitHub workflow schema
- parsed all embedded PowerShell and the shared package validator
- packed real Release and Debug `Opc.Ua.Types` packages and validated
their identities, matching versions, symbols, and `.Debug` family
- verified all seven signing secret names and removal of temporary key
material

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d639a224-b97c-4dbf-ba98-f99e6e3f6b9a
 2026-08-26 16:16:55 +00:00
Copilot cb50353b40 Converting code to new source format for configuration loading (#4311) 
Pull request created by AI Agent

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-24 16:35:31 +00:00
Copilot 496587ef1d Stabilize macOS Redundancy.Server CI by hardening replication wait timeouts (#4309) 
# Description

The `test-macOS-latest-Redundancy.Server` job failed in check run
`97081843689` because
`ReconcileMaterializedValuesCorrectsStaleReplicaAndRebroadcastsAsync`
timed out waiting for inbound replication under runner contention. This
PR makes the timeout OS-aware in that test helper so macOS gets a larger
convergence window without loosening non-macOS behavior.

- **Root cause (from Actions logs)**
- `AwaitWithTimeoutAsync` hit `Task.Delay(30s)` first (`replication did
not complete within the timeout`) in
`ReplicatedAddressSpaceSynchronizerTests`.

- **Targeted change**
  - Replaced hardcoded `30s` waits with a shared helper:
    - macOS: `90s`
    - others: `30s`
- Applied to both eventual-assert deadline and `Task.WhenAny(...)`
timeout in the same test class.

- **Code shape**
```csharp
private static TimeSpan GetReplicationTimeout()
{
    return OperatingSystem.IsMacOS() ? TimeSpan.FromSeconds(90) : TimeSpan.FromSeconds(30);
}
```

## Related Issues

Investigates and addresses CI failure in GitHub Actions check run
`97081843689` (`test-macOS-latest-Redundancy.Server`).

## Checklist

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

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-08-24 16:11:25 +00:00
romanett 24399f1808 Fix source generator MODELGEN003 diagnostic dropping message placeholders (#4310) 
## Problem

`LoggerAdapter.Log<TState>` in
`Tools/Opc.Ua.SourceGeneration/SourceGeneratorTelemetry.cs` reported an
**already fully-formatted** logger `message` against a looked-up
`DiagnosticDescriptor` whose `messageFormat` may declare more positional
placeholders than the single formatted string carries.

For `EventId == 3` the descriptor is `SourceGenerator.Exception`
(`MODELGEN003`) with:

```
messageFormat = "Exception during model generation '{0}': {1}"
```

That format has **two** placeholders but only **one** argument
(`message`) was supplied. Roslyn's `Diagnostic.GetMessage()` runs
`string.Format(format, args)`, which throws `FormatException` because
`{1}` has no argument; Roslyn's internal catch then returns the **raw,
unsubstituted template**. The user therefore saw the literal text
`Exception during model generation '{0}': {1}` with the placeholders
unreplaced, masking the real error.

## Fix

Route the pre-formatted message through a new
`CreateFormattedDiagnostic` helper that reports it via a **passthrough
descriptor whose `messageFormat` is exactly `"{0}"`**, while preserving
the original descriptor's `Id`, `Title`, `Category`, `DefaultSeverity`,
`IsEnabledByDefault`, `HelpLinkUri` and custom tags:

```csharp
public static Diagnostic CreateFormattedDiagnostic(
    DiagnosticDescriptor descriptor,
    string message)
{
    var passthrough = new DiagnosticDescriptor(
        descriptor.Id,
        descriptor.Title,
        "{0}",
        descriptor.Category,
        descriptor.DefaultSeverity,
        descriptor.IsEnabledByDefault,
        helpLinkUri: descriptor.HelpLinkUri,
        customTags: descriptor.CustomTags.ToArray());
    return Diagnostic.Create(passthrough, Location.None, message);
}
```

`LoggerAdapter.Log` now calls this helper instead of
`Diagnostic.Create(descriptor, Location.None, message)`. This renders
the message verbatim under the correct diagnostic id regardless of the
descriptor's placeholder arity, and is safe even when the message itself
contains `{`/`}` characters (the format string is only `"{0}"`).

> Note: the public `descriptor.CustomTags` (`IEnumerable<string>`) +
`.ToArray()` is used because Roslyn's `ImmutableCustomTags` property is
`internal`.

### Why the helper lives in `SourceGeneratorHelpers.cs` (not
`SourceGenerator.cs`)

`SourceGeneratorTelemetry.cs` and `SourceGeneratorHelpers.cs` are shared
(glob-linked) into **both** the model generator
(`Opc.Ua.SourceGeneration`) and the stack generator
(`Opc.Ua.SourceGeneration.Stack`). The stack generator has its **own**
`SourceGenerator` partial class and does **not** compile the model
generator's `SourceGenerator.cs`. Placing the helper in
`SourceGenerator.cs` broke the stack build with `CS0117:
'SourceGenerator' does not contain a definition for
'CreateFormattedDiagnostic'`. `SourceGeneratorHelpers.cs` is the shared
partial (where `Guard` also lives), so the helper is available to both
generators.

### Guard path untouched

The other emission path, `SourceGenerator.Guard(...)` in
`SourceGeneratorHelpers.cs`, uses the same `Exception` descriptor but
correctly passes **two** args (`ex.Message`, `ex.ToString()`), so that
path was already fine and is **not** changed.

## Tests

New
`tests/Opc.Ua.SourceGeneration.Tests/SourceGeneratorTelemetryTests.cs`
(4 tests):

1. **`DirectReportAgainstMultiPlaceholderDescriptorLosesMessage`** —
documents the pre-fix behaviour: the raw `Diagnostic.Create(Exception,
..., "boom")` render drops `"boom"` and leaks `{1}`.
2. **`CreateFormattedDiagnosticRendersMessageVerbatim`** — the fix
renders `"boom"` exactly, with no `{0}`/`{1}`.
3. **`CreateFormattedDiagnosticPreservesDescriptorMetadata`** — id
(`MODELGEN003`), severity, category, help link, IsEnabledByDefault and
custom tags are preserved.
4. **`CreateFormattedDiagnosticIsSafeForBracesInMessage`** — a message
containing `{`/`}` is emitted verbatim.

## Validation

- `tools/Opc.Ua.SourceGeneration` (and the stack variant) build with **0
warnings / 0 errors**.
- Full `Opc.Ua.SourceGeneration.Tests` suite on net10.0: **132/132
passing**, including the 4 new tests.

## Files changed

- `tools/Opc.Ua.SourceGeneration/SourceGeneratorHelpers.cs` — added
`using System.Linq;` + `CreateFormattedDiagnostic` helper.
- `tools/Opc.Ua.SourceGeneration/SourceGeneratorTelemetry.cs` —
`LoggerAdapter.Log` calls the helper.
- `tests/Opc.Ua.SourceGeneration.Tests/SourceGeneratorTelemetryTests.cs`
— new regression tests.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-24 15:19:26 +00:00
Marc Schier 06ae192084 Fix CI failures tracked by #4299 (#4308) 
## Summary
- stabilize the History, authentication lockout, Robotics, redundancy,
and DI tests reported in #4301-#4305
- fix the closed-transport buffered-write completion deadlock that hung
`ConnectMultipleSessionsAsync`
- add macOS crash diagnostics for #4306 and retain evidence for
root-cause analysis
- run macOS tests and NativeAOT in GitHub Actions pull requests, using
supported Intel and ARM64 runners, while keeping every Azure macOS
definition disabled by default
- move all remaining source-level `InternalsVisibleTo` declarations into
project files so signed OpenUSD builds receive the repository public key

## Local validation
- MCP PubSub: 29 passed
- History focused tests: 3 passed
- Sessions lockout: 3 passed
- Sessions multiple-session regression: 3 passed
- deterministic binary client channel tests: 22 passed
- Robotics runtime: 34 passed
- Redundancy focused tests: 4 passed
- OpenUSD net10.0: 914 passed
- affected OpenUSD projects build on net10.0 and net48

Refs #4299, #4300, #4306

Fixes #4301
Fixes #4302
Fixes #4303
Fixes #4304
Fixes #4305

---------

Copilot-Session: 7cb486de-8b4c-43c5-a900-e7d2e0ad04a8
 2026-08-22 19:30:19 +00:00
Copilot 8ea319d910 Fix macOS ARM64 NativeAOT linker failure in aot-macos-latest (#4298) 
# Description

The `aot-macos-latest` job was failing while linking
`tests/Opc.Ua.Aot.Tests` on `macos-latest` / `osx-arm64`, before managed
tests started. The failure was an Apple linker assertion (`too many
large addends`) caused by the size/fixup profile of the NativeAOT test
binary.

- **Root cause**
  - `Opc.Ua.Aot.Tests` publishes a large NativeAOT executable.
- On Apple Silicon CI, the final native link hit the linker’s
addend/fixup limit.

- **Change**
- Keep the fix scoped to
`/tests/Opc.Ua.Aot.Tests/Opc.Ua.Aot.Tests.csproj`.
- For macOS only, make the AOT publish size-focused and drop native
debug / debugger / stack-trace payload:
    - `OptimizationPreference=Size`
    - `DebugType=none`
    - `DebuggerSupport=false`
    - `StackTraceSupport=false`
  - Retain identical-method-body folding.

- **Impact**
  - Lowers linker pressure on macOS ARM64 CI.
  - Leaves production assemblies unchanged.
- Keeps the managed test surface intact; only the CI-only NativeAOT test
executable shape changes on macOS.

```xml
<PropertyGroup Condition="$([MSBuild]::IsOSPlatform('OSX'))">
  <IlcFoldIdenticalMethodBodies>true</IlcFoldIdenticalMethodBodies>
  <OptimizationPreference>Size</OptimizationPreference>
  <DebugType>none</DebugType>
  <DebuggerSupport>false</DebuggerSupport>
  <StackTraceSupport>false</StackTraceSupport>
</PropertyGroup>
```

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

## Checklist

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

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-21 15:44:38 +00:00
Copilot de3476d332 Remove HTTPS binding dependency from server packages (#4297) 
# Description

`Opc.Ua.Server` and `Opc.Ua.Gds.Server.Common` depended directly on
`Opc.Ua.Bindings.Https`, which pulled ASP.NET Core into server
applications
even when optional HTTPS-based transports were not used.

- Remove the HTTPS binding project references from Server and GDS
entirely.
- Add a Core-owned `IOpcUaTransportBuilder` contract for builders that
support
  transport registration.
- Move the type-preserving HTTPS, WSS, Kestrel OPC TCP, and Web API
fluent
  extensions into `Opc.Ua.Bindings.Https`.
- Remove the repository-wide `NoHttps` / `NOHTTPS` build switch.
- Keep ConsoleReferenceServer HTTPS/WSS support through an explicit
binding
  reference and the ASP.NET Core container runtime.
- Document that applications must reference the optional binding package
  directly.

## Validation

- Built `Opc.Ua.Bindings.Https` in Release for all target frameworks.
- Built Server and GDS for .NET 10 and .NET Framework 4.8.
- Built ConsoleReferenceServer and PumpDeviceIntegrationServer for .NET
10.
- Passed 65 HTTPS binding dependency-injection tests.
- Passed focused Server and GDS transport-builder tests.
- Verified refreshed Server/GDS restore assets and output manifests
contain no
  `Opc.Ua.Bindings.Https` dependency.

## Related Issues

Fixes #4296

## 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.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce build or analyzer
warnings.
- [ ] I ran all tests locally using the `UA.slnx` solution against .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: romanett <7413710+romanett@users.noreply.github.com>
Copilot-Session: 41b98d43-8bb8-4587-8f3b-9532544dd582
 2026-08-21 07:29:18 -07:00
romanett afbd10e3e3 Expose runtime capability probes to fix TFM-skew test failures (#4282) (#4295) 
# Description

The nightly `Test .NETStandard 2.1 Tests (net8.0)` leg compiles the
stack as `netstandard2.1` but runs the test assemblies as `net8.0`.
Features gated with `#if NET8_0_OR_GREATER` / `#if NET5_0_OR_GREATER` in
the stack are therefore absent at runtime, while the tests assume they
exist because a `#if` in a test reflects the *test's* TFM (always net8.0
on this leg), not the stack's compiled TFM. This causes the affected
tests to fail on that leg.

This PR fixes it by exposing stack-side runtime capability probes. Each
probe lives in the same assembly as the feature it gates, so its `#if`
reflects that assembly's actual compilation. Affected tests now
`Assert.Ignore(...)` or branch based on the probe instead of relying on
their own compile-time `#if`.

## Approach

New public probes:

- `DtlsRecordProtection.IsAeadSupported` (+ `IsAeadCipherSuite` made
public) - Opc.Ua.PubSub.Udp, net8 gate
- `DtlsEcdheKeyExchange.IsRawSharedSecretSupported` - Opc.Ua.PubSub.Udp,
net8 gate
-
`AdditionalEntropyCertificateKeyGenerator.IsEccKeyRegenerationSupported`
- Opc.Ua.Server, net5 gate
- `HttpsTransportListener.IsWssTransportSupported` -
Opc.Ua.Bindings.Https, `NET5_0_OR_GREATER || NETFRAMEWORK` gate (false
only for the netstandard2.1 build)

Test updates:

- DTLS record/handshake/ECDHE tests skip AEAD/raw-shared-secret cases
when unsupported, preserving HMAC-suite coverage in parametrized tests.
- Server ECC key-regeneration tests skip when unsupported.
- The GDS push tests now *validate* the `BadNotSupported` result at
runtime via the probe rather than compiling out the assertion, which
also preserves the prior net48 behavior.
- The ExperimentalAttribute test probes the stack assembly's
`TargetFrameworkAttribute` at runtime instead of a meaningless
compile-time flag.
- The three WSS integration fixtures skip when WSS is unavailable on the
loaded runtime.

## Notes for reviewers

- On netstandard2.1 the `DtlsRecordProtection` constructor does not
throw for AEAD suites; the `NotSupportedException` surfaces later in
Seal/Open, so the skip is keyed on the cipher suite being AEAD *and*
`!IsAeadSupported`.
- The WSS probe gate returns false only for the netstandard2.1 build
(which defines neither `NET5_0_OR_GREATER` nor `NETFRAMEWORK`),
precisely isolating the build whose legacy Kestrel hosting cannot accept
WebSocket connections on a modern runtime.

Validated with clean builds (0 warnings, 0 errors) for the changed
source projects and for PubSub.Udp.Tests, Server.Tests, Sessions.Tests
and Gds.Tests on both net8.0 and net48.

## Related Issues

- Fixes #4282

## Checklist

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

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-21 06:24:46 -07:00
Copilot 4d71669487 Stabilize macOS Redundancy sample failover tests by isolating demo UDP endpoint (#4292) 
# Description

`test-macOS-latest-Redundancy.Samples` was failing because the PubSub
demo process exited before emitting the expected failover marker,
causing both short-haul demo assertions to time out. This PR hardens the
test harness by isolating demo traffic from the default 4840 discovery
behavior in constrained CI networking.

- **Root-cause containment**
- Set `PUBSUB_ENDPOINT=opc.udp://127.0.0.1:4841` in
`SampleTestEnvironment.FastDemo` so short-haul demo tests run on a
dedicated loopback endpoint.
- Keep `HA_INSECURE=true` in the same preset to preserve demo startup
behavior.

- **Regression guard**
- Added
`SampleTestEnvironmentTests.FastDemoUsesLoopbackEndpointAndInsecureDemoKey`
to lock the test contract for both `PUBSUB_ENDPOINT` and `HA_INSECURE`.

- **Code snippet**
  ```csharp
  public static IReadOnlyDictionary<string, string> FastDemo { get; } =
      new Dictionary<string, string>(StringComparer.Ordinal)
      {
          ["DEMO_FIRST_SECONDS"] = "1",
          ["DEMO_SECOND_SECONDS"] = "1",
          ["PUBSUB_ENDPOINT"] = "opc.udp://127.0.0.1:4841",
          ["HA_INSECURE"] = "true"
      };
  ```

## Related Issues

CI check-run remediation for failing job
`test-macOS-latest-Redundancy.Samples` (job `96263875760`).

## Checklist

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

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [x] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-08-21 04:36: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
romanett b7331a11f2 Fix macOS unicast loopback UDP PubSub transport failures (#4285) (#4290) 
## Summary

Fixes #4285 — on macOS the `Opc.Ua.Tools.Tests` PubSub runtime-tool
tests failed with `SocketException` ("Socket is already connected" on
the publisher side, "No route to host" on the subscriber side) when
using unicast loopback UDP (`opc.udp://127.0.0.1:{port}`).

## Root cause

`UdpDatagramTransport.BindForUnicast` bound sockets to `IPAddress.Any`
(`0.0.0.0` / `::`) and then either `Connect`'d or `SendTo`'d a loopback
destination. macOS (Darwin) is stricter than Windows/Linux about routing
loopback traffic from a wildcard-bound socket:

- **Publisher (connected send):** `Connect(127.0.0.1)` after
`Bind(0.0.0.0:0)` throws `EISCONN` ("Socket is already connected").
- **Subscriber (receive / bidirectional):** binding to `0.0.0.0` and
exchanging datagrams with `127.0.0.1` fails with "No route to host".

## Fix

In `src/Opc.Ua.PubSub.Udp/UdpDatagramTransport.cs`:

- Add `SelectUnicastBindAddress()` — when the endpoint address is a
loopback address, bind to the matching loopback address (`127.0.0.1` /
`::1`) so traffic is routed over the loopback interface. Non-loopback
endpoints keep binding to the wildcard address so datagrams can still
arrive on any interface (no behavior change off loopback).
- Guard `socket.Connect(...)` against `SocketError.IsConnected`
(EISCONN) and treat it as a benign already-connected socket (logged at
Debug), as suggested in the issue.

This is platform-agnostic: on Windows/Linux the loopback bind behaves
identically, and on macOS it removes both socket errors.

## Testing

- `dotnet build src/Opc.Ua.PubSub.Udp` — clean (0 warnings, 0 errors).
- `dotnet test tests/Opc.Ua.PubSub.Udp.Tests`
(`UdpDatagramTransportUnicastTests`) — the existing loopback unicast
publish/receive integration tests pass on net10.0.

macOS validation still relies on CI (change authored on Windows), but
the binding change directly targets the reported Darwin behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-20 17:01:01 +00:00
romanett 2a4da11a9f Fix SourceGeneration tests to probe actual Opc.Ua.Server TFM output folder (#4293) 
# Description

`ModelDependencyScannerTests.GetStackReferences()` reconstructed the
`Opc.Ua.Server` output path from the **test assembly's own TFM**
(`bin/Release/net8.0`). On the netstandard2.1 nightly leg the tests run
as `net8.0` while `Opc.Ua.Server` is built as `netstandard2.1`, so the
hardcoded `net8.0` folder does not exist and every stack-compilation
test threw `System.IO.DirectoryNotFoundException`. Because this only
happens on a nightly-only leg, it was not caught by per-commit CI and
remained on `master`.

The fix stops assuming the test's TFM. A new
`ResolveServerOutputFolder()` helper probes
`src/Opc.Ua.Server/bin/<config>` for whichever TFM subfolder actually
exists, preferring an exact match to the test TFM, then falling back to
`netstandard2.1`, `netstandard2.0`, or any subfolder containing
`Opc.Ua.Server.dll`. It also throws a clear, actionable message when the
server has not been built yet.

This is test-only; no product code changes. Build passes with 0 warnings
and the previously failing tests pass locally.

## Related Issues

- Fixes #4283

## Checklist

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

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [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.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-20 14:42:20 +00:00
romanett 2b06b637f6 Fix MissingMethodException on Crdt init setters for netstandard2.1 build (#4294) 
# Description

The nightly `Test .NETStandard 2.1 Tests (net8.0)` leg builds
`Opc.Ua.Redundancy.Server` as `netstandard2.1` but runs it on the net8.0
runtime, where `Opc.Ua.Redundancy.Server.Tests` failed with
`MissingMethodException: Void
Crdt.CrdtReaderOptions.set_MaxCollectionCount(Int32)`. Because this only
runs nightly, per-commit CI never caught it and it is still present on
`master`.

**Root cause.** `ReplicatedGossipOptions.CreateReaderOptions()` uses an
object initializer on `Crdt.CrdtReaderOptions`, whose
`MaxCollectionCount` / `MaxStringBytes` / `MaxDepth` are `init`-only
setters. Confirmed by reflecting on Crdt 1.1.2:

- The **netstandard2.1** Crdt assembly defines its own internal
`System.Runtime.CompilerServices.IsExternalInit` (netstandard has no BCL
one), and its `init` setters carry a `modreq` to that in-assembly type.
- The **net8.0** Crdt assembly's setters carry a `modreq` to the BCL
`System.Runtime` `IsExternalInit`.

When the netstandard2.1-compiled `Opc.Ua.Redundancy.Server` is loaded on
net8.0, the net8.0 Crdt is loaded at runtime, so the `modreq` the
compiler baked into our `init` call (pointing at Crdt's in-assembly
`IsExternalInit`) no longer resolves, producing the
`MissingMethodException`. This is not version drift (Crdt is pinned to
1.1.2); it is a cross-TFM `init`/modreq identity mismatch.

**Fix.** TFM-guard the setter assignment in
`ReplicatedGossipOptions.CreateReaderOptions()`:

- On `netstandard`, set the three limits via reflection
(`PropertyInfo.SetValue` binds by name and ignores the modreq, so it
resolves against whichever Crdt build is actually loaded).
- On net TFMs (net8/9/10, including the AOT-compiled build), keep the
plain object initializer so the build stays reflection-free.

No production behavior changes on the net runtimes; only the netstandard
build path is affected.

## Related Issues

- Fixes #4284

## Verification

- netstandard2.1 leg (`CustomTestTarget=netstandard2.1`, runs on
net8.0): 12/12 `ReplicatedOptionsTests` pass (previously failing).
- Default net10.0 path: 12/12 pass.

## Checklist

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

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-20 07:23:05 -07:00
Copilot ef667a1082 fix(aot): remove deprecated -ld_classic linker flag causing macOS NativeAOT build crash (#4291) 
The `aot-macos-latest` CI job was crashing during NativeAOT link with an
assertion in `ld_classic` itself (`Assertion failed: (it !=
_dylibToOrdinal.end())`). The `-ld_classic` flag was originally added as
a workaround for a "too many large addends" linker issue, but recent
Xcode toolchains on `macos-latest` runners have a bug in `ld_classic`
that causes it to crash when linking large NativeAOT binaries. The
classic linker is also deprecated and being removed.

## Changes

- **`tests/Opc.Ua.Aot.Tests/Opc.Ua.Aot.Tests.csproj`**: Removed the
`<LinkerArg Include="-ld_classic" />` item for macOS builds. The modern
default linker handles the binary correctly on current toolchains.
`IlcFoldIdenticalMethodBodies` is retained as it still reduces binary
size and fixup count.

## Related Issues

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-20 06:31:52 -07:00
Copilot a3e95de31d Fix UDP multicast discovery announcements failing in restricted network environments (macOS CI) (#4279) 
`UdpDatagramTransport.SendDiscoveryOnceAsync` unconditionally sends
startup discovery announcements to the OPC UA standard multicast address
`224.0.2.14:4840`, even when the connection uses a unicast/loopback
endpoint. In CI environments where multicast is unavailable, this throws
a `SocketException` that propagates through `EnableAsync`, failing the
entire publisher/subscriber startup — and thus the
`test-macOS-latest-Tools` job (`SocketError.HostUnreachable` on macOS,
`SocketError.AccessDenied` on Linux).

## Changes

- **`UdpDatagramTransport.SendDiscoveryOnceAsync`** — wraps the
multicast `SendOnceAsync` calls in a try/catch that swallows
`SocketException` variants indicating multicast is unavailable
(`HostUnreachable`, `NetworkUnreachable`, `NetworkDown`,
`AddressNotAvailable`, `AccessDenied`) and emits a warning log instead
of rethrowing. Multicast discovery is best-effort per OPC UA Part 14
§7.3; it is not required for unicast publisher/subscriber operation.
- **`UdpDatagramTransportLog`** — adds
`MulticastDiscoveryAnnouncementFailed` (event ID 17, `LogLevel.Warning`)
following the existing `[LoggerMessage]` pattern in the file.

```csharp
catch (SocketException ex) when (
    ex.SocketErrorCode is SocketError.HostUnreachable
        or SocketError.NetworkUnreachable
        or SocketError.NetworkDown
        or SocketError.AddressNotAvailable
        or SocketError.AccessDenied)
{
    m_logger.MulticastDiscoveryAnnouncementFailed(ex, m_connection.Name);
}
```

## Related Issues

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] 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: romanett <7413710+romanett@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-08-19 23:43:32 +00:00
Copilot 4ac6084fe5 Fix aot-macos-latest CI: resolve "too many large addends" linker assertion on macOS ARM64 (#4278) 
The `aot-macos-latest` CI job was crashing during the NativeAOT link
step on macOS ARM64 with an Apple linker assertion failure (`ld:
Assertion failed: (_addend == uniqueIndex && "too many large
addends")`). This occurs when the linked binary exceeds the new Apple
`ld`'s fixup/relocation capacity — `Opc.Ua.Aot.Tests` references nearly
the entire SDK surface and has crossed that threshold.

## Changes

- **`tests/Opc.Ua.Aot.Tests/Opc.Ua.Aot.Tests.csproj`** — two
complementary macOS-only workarounds:
- `IlcFoldIdenticalMethodBodies=true`: reduces total fixups by
deduplicating identical method bodies at the ILC stage, lowering the
relocation count below the Apple `ld` threshold.
- `<LinkerArg Include="-ld_classic" />`: falls back to the classic Apple
linker which does not have this assertion, as a secondary safety net.

```xml
<PropertyGroup Condition="$([MSBuild]::IsOSPlatform('OSX'))">
  <IlcFoldIdenticalMethodBodies>true</IlcFoldIdenticalMethodBodies>
</PropertyGroup>
<ItemGroup Condition="$([MSBuild]::IsOSPlatform('OSX'))">
  <LinkerArg Include="-ld_classic" />
</ItemGroup>
```

Both properties are conditioned on `OSX` and have no effect on Windows
or Linux builds.

## Related Issues

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [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: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-19 22:59:25 +00:00
Copilot cde1b85cc9 Fix failing test-macOS-latest-Redundancy.Samples: add HA_INSECURE=true to FastDemo environment (#4280) 
`PubSubDemoHotShowsFailoverContinuityAsync` and
`PubSubDemoColdShowsDataLossAsync` were timing out because the demo
process exited immediately — before printing the `FAILOVER:` line the
tests wait for.

**Root cause**: `SampleTestEnvironment.FastDemo` did not set
`HA_INSECURE=true`. Without it, `CreateRecordProtector` in
`RedundantPubSub/Program.cs` throws `InvalidOperationException` (neither
`HA_RECORD_KEY` nor `HA_INSECURE=true` was present in the child process
environment), terminating the process before any HA narrative output.

**Fix**:

- Added `["HA_INSECURE"] = "true"` to `SampleTestEnvironment.FastDemo`,
matching the same pattern already used in `RedundantServerCluster.cs`.

```csharp
public static IReadOnlyDictionary<string, string> FastDemo { get; } =
    new Dictionary<string, string>(StringComparer.Ordinal)
    {
        ["DEMO_FIRST_SECONDS"] = "1",
        ["DEMO_SECOND_SECONDS"] = "1",
        ["HA_INSECURE"] = "true"   // ← added: prevents immediate InvalidOperationException
    };
```

## Related Issues

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-19 14:05:22 -07:00
romanett c57f31e023 Fix three flaky tests (Robotics, Client, Di) (#4289) 
## Summary
Fixes the three flaky tests surfaced in the last full nightly (ADO build
17152, commit `aad8350`, run
[95295970000](https://github.com/OPCFoundation/UA-.NETStandard/runs/95295970000)).
Each failure has a cross‑platform/cross‑TFM signature (fails on multiple
legs), confirming a race/timing issue rather than a platform bug. All
changes are **test-only** — no production behavior changes.

## Fixes

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

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

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

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

Closes #4286
Closes #4287
Closes #4288

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-19 18:14:53 +00:00
Copilot 91cbf9ca10 Fix PollingWotSubscription: loop stops after correct number of failures when MaxAttempts is set (#4281) 
`PollingWotSubscription.RunAsync` called `GetDelay(attempt)` with the
**pre-increment** failure count, causing the retry policy's "give up"
threshold to be evaluated one iteration late. With `MaxAttempts=1` the
loop would fire a second poll before stopping, failing the
`APolicyThatGivesUpStopsTheLoop` CI test on macOS.

## Change

**`src/Opc.Ua.WotCon.Bindings/PollingWotSubscription.cs`**
- In `RunAsync`, increment `m_consecutiveFailures` **before** calling
`GetDelay` so the policy sees the updated attempt count:

```csharp
// Before — GetDelay sees pre-increment value; MaxAttempts=1 needs 2 failures to stop
int attempt = Volatile.Read(ref m_consecutiveFailures);
delay = m_retryPolicy.GetDelay(attempt);
Volatile.Write(ref m_consecutiveFailures, attempt + 1);

// After — GetDelay sees the incremented value; MaxAttempts=1 stops after 1 failure
InterLocked.Increment(ref m_consecutiveFailures) ;
delay = m_retryPolicy.GetDelay(attempt);
```

`ExponentialBackoffChannelReconnectPolicy.GetDelay` returns a negative
delay (stop signal) when `attempt >= MaxAttempts`, so the off-by-one
meant `MaxAttempts=N` previously required `N+1` failures.

## Related Issues

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] 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.
- [x] 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-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>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-19 17:17:49 +00:00
Copilot 659d3c2985 Fix "Build, sign, pack, publish" CI failure: register new Aas NuGet packages (#4277) 
The `validate-source-generator-packages.ps1` script failed because four
new AAS packages introduced to the build were not registered in the
expected-packages list, causing the pipeline to treat them as accidental
leaks.

## Changes

- **`.azurepipelines/expected-packages.txt`**: Added the four new AAS
package IDs in alphabetical order:
  - `OPCFoundation.NetStandard.Opc.Ua.Aas`
  - `OPCFoundation.NetStandard.Opc.Ua.Aas.Client`
  - `OPCFoundation.NetStandard.Opc.Ua.Aas.Server`
  - `OPCFoundation.NetStandard.Opc.Ua.Aas.WoT`

## Related Issues

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-19 07:57:59 -07: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 397db603db Add OPC UA for Asset Administration Shell: the published OPC 30270 and v3 draft (#4244) 
# Description

Implements the **Asset Administration Shell** companion specifications
on top of `marcschier/wot-type-binding`, in two halves that a Server
chooses between:

* **AAS V3** — the
[draft](https://github.com/marcschier/opcua-drafts/tree/main/companion-specs/AAS)
(`http://opcfoundation.org/UA/I4AAS/v3/`), all sixteen conformance
units: the metamodel, the registry built on the abstract xRegistry base,
generational materialization, federation, packages with digest-bound
integrity, the Digital Product Passport vocabulary, and the Annex F
bridge to WoT Thing Descriptions.
* **AAS V2** — the published [OPC
30270](https://reference.opcfoundation.org/specs/OPC-30270/full)
(`http://opcfoundation.org/UA/I4AAS/`), which maps the AAS V2.0.1
metamodel: object model, ingestion of XML, JSON and AASX,
materialization, server and client.

The two are different metamodel generations rather than two revisions of
one model — V2 keeps `Asset` as a first-class Identifiable,
`identification.idType`, Views and ordered collections, none of which
survive into V3 — so they are **alternatives**. A Server hosts one or
the other; this is documented rather than enforced at startup. Each is
named for the generation it maps (`Opc.Ua.Aas.V2`, `Opc.Ua.Aas.V3`,
`AddAasV2Server`, `AddAasV3Server`), and what genuinely serves both
carries no marker and sits at the root of `Opc.Ua.Aas`: the
deterministic NodeId encoding and idShortPath conventions,
`AasOptional<T>`, the digest rule, the materialization diagnostics, the
AASX container walk, and the value and operation providers, which are
keyed on `NodeId` and `Variant` and so carry no version at all.

## What is in here

**New libraries**: `Opc.Ua.Aas`, `Opc.Ua.Aas.Server`,
`Opc.Ua.Aas.Client`, `Opc.Ua.Aas.WoT`, plus `samples/Aas` and
`docs/Aas.md`.

**Enabling changes to existing code**, each needed by the above:

* The OPC UA `Decimal` DataType added to `Opc.Ua.Types`, with its Part 6
§5.1.10 wire encoding. `BinaryDecoder` gained one internal member,
`TryReadRemainingBodyBytes`, because a trailing field with no
self-describing length cannot otherwise be read; `Decimal.Decode` is its
only caller. Part 6 Table 3 types the unscaled `Value` as an
**OctetString whose byte count is `Length` − 2**, not a length-prefixed
`ByteString`, so it carries no length of its own —
`DecimalEncodingTests` pins the body byte-for-byte against that mistake,
and against the `Length ≤ 2` values the same table calls invalid.
* The xRegistry projection engine extracted into
`Opc.Ua.XRegistry.Server` and `Opc.Ua.WotCon.Server` migrated onto it
(−848/+387 lines), with the WoT Connectivity tests passing **unedited**.
* Two source-generator fixes, both of which made a model impossible to
build (see below).

## Defects found and fixed along the way

Running the stack end to end, rather than trusting per-area test suites,
surfaced defects that no unit test caught. The ones worth a maintainer's
attention:

* **The generator emitted non-terminating constructors.** A structure
field with no declared default was initialized with `new T()`, so a
self-referential DataType (`AASReferenceDataType.referredSemanticId`)
recursed until the stack overflowed and `AddOpcUaAas` could not load the
model at all. The test host *died* rather than failing. Only the field
that closes a cycle is now left null.
* **The generator omitted `new` on a redeclared placeholder.**
`AASOrderedSubmodelElementCollectionType` redeclares the
`SubmodelElement` slot of its base type, and the generated `Add` method
hid the base one without saying so, so the V2 model would not compile.
No existing model could have hit either defect — both are build
failures, so anything that builds today is unaffected.
* **The metamodel half had never worked outside its unit tests**: the
emitted NodeSet declared no reference-type aliases and was rejected by
the loader; the Mandatory `Invoke` Method was never emitted, because a
NodeSet is imported as written rather than instantiated from its type;
the runtime bound at the authored namespace index instead of the one the
Server assigns; and `MethodDeclarationId` was addressed in the wrong
namespace. Every materializer test inspected the in-memory object graph,
which cannot see whether the graph is loadable — two tests now import it
exactly as `RuntimeNodeSetNodeManager` does.
* **The client could not see any Property member.** It browsed
`HasComponent` with subtypes, but `HasProperty` is a sibling under
`Aggregates`, so reading a value failed and invoking failed argument
validation.
* **Every repeated registry write forked a duplicate group and
resource** instead of adding a version, because the identifier was
re-derived against a sibling set that already contained the entity
itself. This also made the persistence bounds unreachable.
* `.gitignore`'s `**/packages/*` silently swallowed the whole
package-integrity implementation and its tests, which would have
vanished on a fresh clone.

## Specification findings

Nine findings against the V3 draft are fixed in
[marcschier/opcua-drafts#75](https://github.com/marcschier/opcua-drafts/pull/75).
The substantive one: clause 6.1.5's absent-versus-empty rule was
**unsatisfiable** for five container fields that materialize as parent
components with no node of their own, which made `AAS-LosslessRoundTrip`
unachievable.

OPC 30270 contradicts itself twice. Both are recorded in
`AasV2ConformanceUnits.cs` and the documentation rather than smoothed
over: Table 83 spells the unit `I4AAS MultiLangaugeProperty` where Table
85 spells it correctly, and Table 85 lists an `I4AAS Security` unit that
Table 83 never defines. Table 84, which would assign profile URIs, is
empty, so `ServerProfileArray` gains no I4AAS entry.

## Known limitation

Calling an `Operation` **with arguments** answers `BadTooManyArguments`.
Method resolution works from both the instance NodeId and the
`MethodDeclarationId`, and the argument definitions are published, but
`NodeState.AddChild` does not route through `MethodState.FindChild`, so
a Method imported from a NodeSet leaves its typed `InputArguments` null.
Closing this means changing how the shared NodeSet import claims typed
children, which affects every consumer of runtime NodeSets and is out of
scope here. It is documented in `docs/Aas.md` §11, the conformance
matrix marks `AAS-OperationInvoke` partial rather than claiming it, and
`samples/Aas` exercises the path and reports the status rather than
working around it. V2 is unaffected — its `Operation` is an
OptionalPlaceholder with no declared arguments.

## Related Issues

- No tracking issue yet. Opening as a **draft** for design discussion
first, per the ADR guidance below; happy to open one and link it once
the shape is agreed.

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

### Verification

| Check | Result |
|---|---|
| AAS tests | **670 passed, 0 failed** on net472, net48, net8.0, net9.0,
net10.0 |
| Coverage | Client 80.8 %, Server 81.2 %, `Opc.Ua.Aas` 82.0 %, WoT 82.4
% — all above the 80 % gate |
| Libraries | `Opc.Ua.Aas`, `.Server`, `.Client`, `.WoT` build clean on
every target framework, zero warnings |
| Regression | `Opc.Ua.Types` 8677, `Opc.Ua.WotCon` 1094,
`Opc.Ua.XRegistry` 198, `Opc.Ua.Server` 4050 — all green |
| Sample | `samples/Aas` builds warning-free and runs end to end |
| Merge validation | Solution builds clean on net10.0 and net48; WoT
Types 504, WotCon 1111, AAS 670, XRegistry 198 tests passed |
| CI | All 99 checks green on `ff9298e44`; only the CLA signature status
is outstanding |

### Review pass

A full review against both specifications, and for API consistency and
correctness, produced twelve
fixes. Each one has a test that fails without it.

| Defect | Where | Why it mattered |
|---|---|---|
| V2 projection aborted on **every** `File` read from a document |
`AasV2EnvironmentRuntime` | The runtime bound `File/Value`
unconditionally, the materializer emitted it only with content, and the
builder throws `BadNotFound` for an unknown NodeId — aborting the whole
registration. No reader ever populates `AasFile.File`, so no AAS V2
document could be hosted at all. |
| `aas-suppl` anchored on the wrong part | `AasxPackageReaderCore`,
`AasxPackageWriter` | IDTA and the Package Explorer anchor it on the
environment part; we used the origin part. Every real `.aasx` returned
zero supplementary files, silently. Affects both halves. |
| Federation failed closed on pooled connections | `AasFederation` |
`ConnectCallback` fires only for new connections, so the second fetch to
an origin — and all of net472/net48/netstandard2.1 — failed with a
security-flavoured error. The peer is now validated *inside* the
callback, which makes reuse safe by construction. |
| Malformed `Decimal` escaped as `ArgumentOutOfRangeException` |
`BinaryDecoder` | A declared body shorter than `Scale` produced a
negative read length. Two of the three test cases had been passing for
the wrong reason. |
| V2 and V3 cross-wired their value providers |
`OpcUaAasServerBuilderExtensions` | Both registered `IAasValueProvider`;
whichever was added first served both AddressSpaces. V2 now has its own
contract, so co-hosting is harmless rather than silently wrong.
`EnvironmentFolder` was also bound and ignored on the V2 path. |
| Export filter skipped three nesting kinds | `AasEnvironmentExporter` |
Content under `Entity`, `AnnotatedRelationshipElement` and `Operation`
was exported unevaluated *and* stamped with a Digest presenting it as
complete (clause 6.5.10). List authorization paths also shifted after a
denial. |
| Concealment held at the Method but not in the projection |
`AasRegistryModel` | `GetSubmodel` answered `BadNotFound` while an
anonymous Browse revealed the node, its identifier, semanticId and
digest (clause 6.5.7). |
| File handles were unsynchronized and unowned | `AasElementFileManager`
| The async node manager holds no lock, so concurrent Calls raced a bare
`Dictionary`; sequential handles let any Session read, seek or commit
another's. Writes were also unbounded. |
| Projection host was unimplementable outside the stack |
`AasEnvironmentProjection` | The handle demanded a type with an internal
constructor — our own tests used
`FormatterServices.GetUninitializedObject` to get round it. Now mirrors
WotCon's interface-typed registration. |
| Embedded `FileType` omitted 7 of 10 Mandatory members |
`AasEnvironmentMaterializer` (V2) | OPC 10000-3 6.4.4 requires them all;
without `Size` and `OpenCount` a Client cannot size a read or detect an
abandoned handle. |
| Round-trip oracle ignored most of the model | `AasRoundTripComparer` |
`Blob`, `File`, `ReferenceElement` and `BasicEventElement` had no case
arm, so their fields compared equal by falling off the switch;
`valueType`, `semanticId`, `entityType` and `Key.type` were never read.
The clause 6.4 losslessness claim rested on it. |

Known, unchanged: calling an `Operation` with arguments answers
`BadTooManyArguments`, because
`NodeState.AddChild` does not route through `MethodState.FindChild`.
Documented in `docs/Aas.md` §11.

Not addressed in this pass, and worth a maintainer decision: OPC 30270
§9.2 says all NodeIds of
Nodes not defined in the companion specification **shall not** use the
standard namespaces, and
Table 87 marks a vendor-specific instance namespace Mandatory. Both
halves currently mint instances
in the companion namespace. Separating them touches NodeId minting, the
runtime and the client
together, so it is better done deliberately than folded into a review
pass.
This is a large change and touches shared code (`Opc.Ua.Types`, the
source generator, the xRegistry projection). It is deliberately a draft:
the enabling changes are separable if maintainers would prefer them
split into their own PRs. The latest `master` is merged into this branch
in `0dd0ff0e1`.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
Copilot-Session: c3d88950-4a65-4ff6-87d2-1f545a875c9b
Copilot-Session: b8ab3763-b9bd-46ae-b0ef-045db26e5782
 2026-08-18 08:11:47 -07:00
romanett 8634a18884 Simplify server shutdown teardown (#4262) 
# Description

Server shutdown had grown into roughly 500 lines of coordination
machinery in `StandardServer.OnServerStoppingAsync` and
`ServerBase.StopAsync`: a `ServerShutdownState` state machine,
request-admission-close gates, lifecycle-waiter drain coordination,
multi-stage base-resource disposal, and a large set of `ForTest` hooks.
Most of that existed to guarantee no request was still dispatching into
the address space when it was torn down. That guarantee is now provided
directly by disposing the request queue, so the surrounding machinery is
no longer needed.

## Approach

- **Run once, re-armed on restart.** `ServerBase.StopAsync` now uses a
shared-task guard (`m_stopTask ??= StopCoreAsync(...)` under a
`System.Threading.Lock`), so concurrent and repeat callers await the
same completion. `InitializeRequestQueue` (which already runs on every
start) resets the guard, re-arming shutdown for a restarted server.
- **Drain via the request queue.** Teardown disposes the request queue
first through the new `StopRequestQueue()`, which cancels in-flight
processing and waits for the workers before any NodeManager is stopped.
`RequestQueue.Dispose` is now idempotent because the queue can be
disposed both by shutdown and later by base disposal / re-init.
- **Linear teardown.** `OnServerStoppingAsync` is now short and linear:
shutdown delay, stop the registration timer, unregister from discovery,
then an ordered teardown (lifecycle begin, subscription manager, session
manager, node managers, lifecycle complete, dispose) that collects
per-stage failures into an `AggregateException`.
`Dispose`/`DisposeAsync` reduce to the same shared-task pattern.
- **Trim the lifecycle extension.** Removed the shutdown-only
request-admission members from `RequestManager` and
`RequestManagerLifecycleExtension` (admission close, admission
validation, repeat-until-idle drain). The lifecycle-waiter machinery
that live NodeManager replacement (`NodeManagerLifecycle`) still depends
on is kept intact.

Net effect is about 1500 lines removed against roughly 190 added.

## Notes for reviewers

- The drain is cancellation-based rather than a graceful wait: disposing
the queue cancels in-flight request processing and drains queued
requests to `BadServerHalted`. This is the intended behavior for
stop/dispose.
- `RequestManagerLifecycleExtension` is deliberately only partially
trimmed. Its waiter/exclusion logic is shared with
`NodeManagerLifecycle`, so only the members that were exclusive to the
old shutdown path were removed.
- The removed `ForTest` hooks drove three now-deleted test files' worth
of coordination assertions; those tests were rewritten or removed. A
minimal `BaseResourcesDisposedForTest` hook is kept for the rewritten
dispose tests.

## Related Issues

N/A

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [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.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-18 07:05:35 -07:00
romanett e1932152c1 Harden server subscription deletion/transfer race (#4261) 
# Description

Deleting or transferring a server-side subscription concurrently with
monitored-item operations was guarded by an ad-hoc, per-method
`EnsureSessionNotClosing` check plus layers of `...Core` indirection.
That made the actual lifecycle contract hard to reason about and left a
window where a call could still race a delete. This tightens the model
so a subscription has one explicit "deleted" gate and closing-session
teardown is deterministic.

Key changes:

- **Collapse the indirection.** The subscription monitored-item
overloads and `...Core` methods are folded back into the public
interface methods (`CreateMonitoredItemsAsync`,
`ModifyMonitoredItemsAsync`, `DeleteMonitoredItemsAsync`,
`SetMonitoringModeAsync`). `EnsureSessionNotClosing` is removed; these
operations now rely on session ownership via `VerifySession` plus the
new deletion gate instead of an ad-hoc closing-session check.
- **Explicit deletion gate.** Added `ISubscription.IsDeleted`, set as
the first step of `DeleteAsync`. Every publicly callable `Subscription`
method throws `Bad_SubscriptionIdInvalid` once the subscription is
deleted. `DeleteAsync` performs its own inline monitored-item teardown
so it is not blocked by its own gate.
- **Deterministic disposal.** Subscriptions are disposed after
`DeleteAsync` at both `SubscriptionManager` delete sites
(`MonitoredItem.Dispose` is idempotent, so the redundant item teardown
is safe).
- **Idempotent session close.** `ServerInternalData.CloseSessionAsync`
now returns early when `MarkSessionClosing` reports the session was
already closing; `Session.MarkClosing` returns whether it actually
transitioned (via `Interlocked.Exchange`).

One behavior change worth a careful look: because the closing-session
gate is gone, a closing session that still owns a subscription can
complete monitored-item operations. The existing
`CreateMonitoredItemsRejectsClosingSession` test was repurposed to
`CreateMonitoredItemsDoesNotRejectClosingSession` to assert this new
contract. `SubscriptionManager` still guards `CreateSubscription`,
`Publish`, and `TransferSubscriptions` on `IsClosing` with
`Bad_SessionClosed`.

## Related Issues

N/A

## Checklist

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

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. _(Validated
the full `Opc.Ua.Server.Tests` project on net10.0 - 4095 passed, 5
skipped; net48 not yet run.)_
- [ ] 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 App <223556219+Copilot@users.noreply.github.com>
 2026-08-18 07:03:51 -07:00
romanett 333de8305b Add kitchen-sink coverage NodeSet2 with three load pipelines (#4251) (#4272) 
# Description

Regression fence for the NodeSet2 import + code-generation surface. This
adds a single, purpose-authored "kitchen-sink" NodeSet2
(`Opc.Ua.CoverageTest.NodeSet2.xml`) plus a small dependent secondary
model that deliberately flip a broad set of the knobs defined by
`UANodeSet.xsd` (every node class, every `Definition` shape, custom
reference types, all five modelling rules, scalar/array/matrix values, a
deep branching instance tree, and a wide set of reference relations).
The same bytes are then loaded into **real** servers (`ServerFixture<T>`
+ `ReferenceServer`) through three independent pipelines and validated
node-by-node, attribute-by-attribute, reference-by-reference:

1. **Source-generation, hand-written manager** - the XML is consumed at
compile time via `<AdditionalFiles
ModelSourceGeneratorPrefix="Opc.Ua.CoverageTest">`; the generated
`AddOpcUaCoverageTest(context)` composes the predefined nodes inside a
`CustomNodeManager2`, so every node materialises as its generated
`NodeState` subclass.
2. **Runtime import** - the identical XML is imported at startup through
`RuntimeNodeSetNodeManagerFactory` / `AddRuntimeNodeSet`.
3. **Source-generation, generated fluent manager** - a
`[NodeManager]`-attributed partial makes the generator emit the whole
`FluentNodeManagerBase` manager, factory and typed fluent-builder
surface.

A shared, data-driven `[TestCaseSource]` battery runs the identical
assertions against all three servers, and a cross-pipeline equivalence
fixture asserts the three address spaces agree. A dependent **second
namespace** (`.../Secondary/`) exercises cross-namespace
`HasTypeDefinition`, `HasSubtype` and `HasComponent` references in every
pipeline.

Authoring the model surfaced two latent source-generator bugs, both
fixed here:

- `NodeSetToModelDesign` crashed on a top-level `<ServerUris>` table
(`StringTable.Append(null)` for the reserved local-server slot). Added a
null guard plus a focused regression test (`NodesetServerUrisTests`).
- `FluentBuilderGenerator` emitted colliding typed-builder accessors for
any model with more than one structure encoding, because
`DataTypeEncoding` objects hang off their `DataType` and surfaced as
parent-less "root" instances sharing the reserved
`DefaultBinary`/`DefaultXml`/`DefaultJson` symbolic names. They are
encoding metadata, never fluent-wired, so they are now excluded from the
typed builder.

A few edges are intentionally not asserted for cross-pipeline
equivalence because the pipelines legitimately differ (documented in
`CoverageNodeSet/README.md`): type-dictionary / `DataTypeDescription`
nodes (the generator regenerates schema rather than emitting them), a
handful of ambiguous duplicate-hierarchical / View-`Organizes` edges,
and runtime structure stand-ins (structure definitions are validated via
the `DataTypeDefinition` round-trip instead).

## Related Issues

- Fixes #4251

## Checklist

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

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [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.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-17 14:59:58 -07: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