GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-28 14:09:56 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,498 Commits 75 Branches 105 Tags
91cbf9ca10bcfbcc1c1c1759aef344cdc78ca7ae
Commit Graph
2498 Commits
This Branch
This Branch
All Branches
Author SHA1 Message Date
CopilotromanettromanettCopilot Autofix powered by AI
 		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
Copilotandromanett 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 SchierandCopilot 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 SchierandCopilot 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
romanettandCopilot App 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
romanettandCopilot App 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
romanettCopilot Appcopilot-swe-agent[bot]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 SchierCopilotcopilot-swe-agent[bot]marcschier
 		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
romanettandCopilot App d547747e22 Generate ReferenceNodeManager address space from a NodeSet2 model (#4220) 
# Description

The CTT/compliance `ReferenceNodeManager` built its ~6,000 node address
space entirely in imperative C#. This PR migrates it to a
source-generated `[NodeManager]` partial driven by an exported NodeSet2
model (`ReferenceServer.NodeSet2.xml`), mirroring the
`MinimalBoilerServer` sample and completing the goal of issue #4204.

The address space is now materialized from the model by the source
generator. The large orchestration body was kept but converted from
create-based to **find-instead-of-create**: each `Create*` primitive now
looks up the model-materialized node via `FindPredefinedNode<T>` and
re-applies runtime values, DataAccess metadata, and write handlers,
rather than constructing nodes from scratch. This keeps the
deterministic value seeding and behavior wiring intact while the
structure comes from the model.

## Notable details for reviewers

- **Idempotent reference wiring.** The mesh references in the
`References` folder are already present in the model, so the historic
manual `AddReference` calls would throw on duplicates. Added a small
`AddReferenceIfMissing` helper so those calls are safe.
- **Non-string NodeIds.** The `NodeIds` folder assigns numeric / GUID /
opaque NodeIds. The model exports those nodes with their final ids, so
find-by-string-path no longer works; added `CreateVariableById(NodeId)`
to locate them by their real id.
- **Generator bug fix (worth a careful look).**
`ModelDesignExtensions.GetValueRankAsCode` reconstructed `ValueRank` as
`dimensions.Length + 1`, so a node with `ArrayDimensions="3,3"` was
emitted as `ValueRank = 3`. This affected **any** NodeSet2 with a fixed
multi-dimensional array (rank-3 matrices, `BadTypeMismatch` on
whole-matrix writes, wrong decoded dimensions). Corrected to
`dimensions.Length` and updated the unit tests that had codified the
off-by-one.

## Validation

- `ReferenceServerTest`: 44/44 on net10.0 and net48
- `Opc.Ua.SourceGeneration.Core.Tests`: pass on
net8.0/net9.0/net10.0/net48
- Integration on net10.0: ComplexTypes, Client, History, Sessions,
Subscriptions all pass
- Clean build, no new analyzer warnings

## Related Issues

- Fixes #4204

## 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.
- [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-15 05:22:31 -07:00
CopilotmarcschierMarc Schier
 		aad9e9b1c1 Document Opc.Ua.Core.Diagnostics trim/AOT exclusion and suppress reflection IL2075 warnings (#4249) 
## Description

`Opc.Ua.Core.Diagnostics` has two trim/AOT defects that are invisible
today because the assembly is not marked `IsAotCompatible`:
`NicCaptureSource.StartAsync` carries
`[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` that its interface
member `ICaptureSource.StartAsync` lacks (IL2046/IL3051), and
`ServiceCallReassembler` reflects over `IEncodeable` properties by name
(IL2075 ×3), which under trimming silently returns `null` instead of
failing loudly.

### NicCaptureSource / ICaptureSource mismatch
- Left as-is: `ICaptureSource` stays unannotated since most capture
sources don't need the SharpPcap dynamic-loading requirement, and
`NicCaptureSource` genuinely does.
- Documented explicitly in `Opc.Ua.Core.Diagnostics.csproj` that the
project is deliberately **not** `IsAotCompatible`/`IsTrimmable` for this
reason, mirroring the existing pattern in
`Opc.Ua.Mcp.Diagnostics.csproj`.

### ServiceCallReassembler reflection
- Verified the reflection-based property lookup is intentional (avoids
allocating via `ToArray()` just to get a count; covered by existing
tests).
- Added `[UnconditionalSuppressMessage]` with justification and a TODO
to `GetCollectionCount`/`GetPropertyString`, pointing at the
source-generated-accessor approach suggested as the long-term fix,
instead of leaving the reflection silently unannotated.

```csharp
// TODO: Replace these string-keyed property lookups with a source-generated
// accessor ... until then, GetType().GetProperty(...) here is unavoidable
// and is suppressed rather than silently left unannotated.
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
    Justification = "...")]
private static int? GetCollectionCount(IEncodeable message, string propertyName)
```

Verified by temporarily enabling `IsAotCompatible` locally: all 3
IL2075s resolve, leaving only the accepted/documented `NicCaptureSource`
IL2046/IL3051 mismatch.

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

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

- Fixes #4236

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
 2026-08-14 17:41:28 +00:00
CopilotmarcschierMarc SchierCopilot Autofix powered by AI
 		a7d11db1bd Enable fluent subscription option configuration (#4248) 
# Description

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

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

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

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

## Related Issues

## Checklist

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

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

- Fixes #4245

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-08-14 17:40:48 +00:00
romanettandCopilot App 8d2d363d04 Add MultiState Discrete write validation (#4255) 
## Summary

Adds hand-written partial state classes that enforce OPC UA Part 8
§5.3.3 write validation for the `DiscreteItemType` subtypes, plus tests.

## Details

- **`MultiStateDiscreteState`** — a written value is treated as an index
into `EnumStrings`. Returns `BadOutOfRange` when the index is outside
`[0, len)`.
- **`MultiStateValueDiscreteState`** — a written value is matched
against the sparse `EnumValues` set. Returns `BadOutOfRange` when no
entry matches. On a good write, `ValueAsText` is refreshed from the
matching entry's display name.
- **`DiscreteItemState`** — provides the shared `TryGetIntegerValue`
helper that normalizes any scalar integer built-in type to `long` (the
exact-type `Variant` accessors return `default` on a built-in-type
mismatch, so a `BuiltInType` switch is required).
- **`TwoStateDiscreteType`** intentionally has no hand-written class.

These 3 classes are `public partial class` extending the
source-generated DataAccess partials already present on `master`. They
read via the typed properties (`EnumStrings`, `EnumValues`,
`ValueAsText`) rather than `FindChild`, because
`BaseDataVariableState.FindChild` special-cases the `EnumStrings`
BrowseName and never falls through to the `AddChild`'d children.

## Testing

- `dotnet build src\Opc.Ua.Core.Types\Opc.Ua.Core.Types.csproj -c
Release -f net10.0` → 0 warnings, 0 errors.
- 13 tests across `MultiStateDiscreteStateTests` and
`MultiStateValueDiscreteStateTests` pass (13 passed, 0 failed).

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-14 08:24:45 -07:00
romanettandCopilot App aef86ba9dc Add SelectionListType write validation (#4254) 
## Summary

Adds write-validation for the standard OPC UA `SelectionListType`
VariableType via the handwritten `SelectionListState` partial, plus unit
tests. Split out of #4220 into its own PR against `master`.

## The feature

`SelectionListState` overrides `WriteValueAttribute` to enforce **OPC UA
Part 5 section 7.18**: when the optional `RestrictToList` property is
present and `true`, a written value must be one of the entries in the
mandatory `Selections` property.

Behavior contract:
- Validation **gates on `RestrictToList`** — only enforced when the
property is present and true. Otherwise the base-class behavior applies
unchanged.
- **Empty `Selections` rejects all** values (nothing is a valid member).
- Membership is **data-type independent** — determined via `Variant`
equality rather than any specific built-in type.
- **Mismatched built-in types defer to the base class** for its normal
type validation.

The generated `SelectionListState` partial (base
`BaseDataVariableState`, typed
`Selections`/`SelectionDescriptions`/`RestrictToList` properties) is
produced by the source generator from the core address space; this
handwritten partial supplies only the write-validation override, using a
single clean generic helper.

## Validation

- Build `Opc.Ua.Core.Types` (net10.0, Release): **0 warnings, 0
errors**.
- `SelectionListStateTests` (net10.0, Release): **14 passed, 0 failed**.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-14 08:24:34 -07:00
romanettCopilot Appcopilot-swe-agent[bot]romanett
 		6e0ed89d3f Make UANodeSetSerializers.g.cs generation deterministic (#4252) 
## Problem

Regenerating `src/Opc.Ua.Types/Schema/UANodeSetSerializers.g.cs` via
`UANodeSetSerializers.ps1` produced ~144 lines of no-op churn on every
run. The sgen tool (Microsoft.XmlSerializer.Generator) emits the Reader
`idNN_Name` field declarations and the matching `InitIDs()` initializers
in a hash-seed-dependent order: the assigned id numbers are stable, but
their emission order shuffles between runs. This made an otherwise no-op
regeneration produce a large, noisy diff that was painful to review.

## Fix

- Added a canonicalization pass in `UANodeSetSerializers.ps1` that sorts
each of the two affected contiguous blocks (the `string idNN_Name;`
field declarations and the `idNN_Name = Reader.NameTable.Add(...)`
initializers) by numeric id ascending, so re-running on unchanged input
yields byte-identical output.
- The name regex was widened to `@"[^"]*"` so entries with an empty name
(e.g. `id3_Item`) or URI-style names do not split the block into
independently-sorted runs (which would keep it unstable).
- Regenerated the committed `.g.cs` once through the improved script;
the resulting diff is sort-only.

## Also fixed: tool download

Replaced `Expand-Archive` (which rejects the `.nupkg` extension on
Windows PowerShell) with
`System.IO.Compression.ZipFile.ExtractToDirectory`, so a clean run with
an empty tool cache works end-to-end on both Windows PowerShell 5.1 and
PowerShell 7+.

## Validation

- From an empty tool cache, the full download + extract + generate path
succeeds.
- A second run is a byte-identical no-op (`git diff --exit-code` = 0).
- `dotnet build src\Opc.Ua.Types\Opc.Ua.Types.csproj -c Release -f
net10.0` builds with 0 warnings and 0 errors.
- All AOT post-processing is preserved: custom header, `#if
NET5_0_OR_GREATER` wrapper, module IL2026/IL3050 suppressions,
`[RequiresUnreferencedCode]`/`[RequiresDynamicCode]` on the
InitCallbacks() overrides (Writer = RequiresUnreferencedCode only,
Reader = both), pragmas, UTF-8 BOM, and CRLF line endings.

No hand-edits to `.g.cs`; the fix lives entirely in the script so future
regenerations stay stable.

## Also: Load UserAccessLevel from NodeSet2 model

Bundled here because it regenerates `UANodeSetSerializers.g.cs`, which
this PR owns.

- `UANodeSet.cs`: adds the `{Member}Specified` XmlSerializer convention
to `UAVariable.UserAccessLevel` (new `UserAccessLevelSpecified`
companion; removed the `[DefaultValue((uint)1)]`) so an
explicitly-present `UserAccessLevel` attribute is distinguishable from
the schema default of 1.
- `UANodeSetSerializers.g.cs`: regenerated - the writer now emits
`UserAccessLevel` when `UserAccessLevelSpecified`, and the reader sets
`UserAccessLevelSpecified = true`. Regenerating produced only these two
behavioral changes plus removal of pre-existing empty `if
(o.@MemberSpecified) { }` no-op blocks (see below); no `idNN` re-sort,
so determinism held.
- `UANodeSetHelpers.cs`: the runtime importer honors an
explicitly-specified `UserAccessLevel` instead of always deriving it
from `AccessLevel`.
- Source generator (`ModelDesign.cs` `RawUserAccessLevel`,
`ModelDesignExtensions.GetUserAccessLevelAsCode`,
`NodeSetToModelDesign.cs`): emit the verbatim `UserAccessLevel` when the
NodeSet sets it, otherwise derive from `AccessLevel` (matching the
runtime importer).

### Generator normalization for `{Member}Specified`

sgen emits an extra empty `if (o.@MemberSpecified) { }` no-op block near
`WriteEndElement` for every member that uses the `{Member}Specified`
convention. Adding the companion to `UserAccessLevel` would have
introduced one such block, and the committed file already contained
several for other members (`LastModified`, `AccessRestrictions`,
`PublicationDate`). `UANodeSetSerializers.ps1` now strips these empty
guard blocks during post-processing, so the generated file stays free of
dead code and a re-run of the generator remains a byte-identical no-op.
The real, non-empty guard around the `WriteAttribute` call is preserved.

The ReferenceServer sample application of this feature is intentionally
excluded and lands with #4220.

### Validation (UserAccessLevel)

- `dotnet build src\Opc.Ua.Types\Opc.Ua.Types.csproj -c Release -f
net10.0` and `dotnet build
tools\Opc.Ua.SourceGeneration.Core\Opc.Ua.SourceGeneration.Core.csproj
-c Release`: both 0 warnings / 0 errors.
- `dotnet test tests\Opc.Ua.SourceGeneration.Core.Tests -c Release -f
net8.0 --filter
"FullyQualifiedName~NodeSetToModelDesign|FullyQualifiedName~UserAccessLevel"`:
16 passed, 0 failed.
- Re-running the generator remains a byte-identical no-op.

---------

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-14 08:00:13 -07:00
romanettandCopilot App 34fe1a1567 Emit WriteMask from the model in the source generator (#4253) 
## Summary

Isolates a self-contained OPC UA source-generator bug fix, extracted
from PR #4220 (which targets a different base branch) so it can merge
independently against `master`.

## Fix: Emit node WriteMask from the model

**Bug:** The `NodeState` source generator hardcoded
`global::Opc.Ua.AttributeWriteMask.None` for both the `WriteMask` and
`UserWriteMask` attribute replacements. As a result, attributes declared
writable in a NodeSet2 model (for example `WriteMask=96`) were emitted
as `None`, so writes to those attributes failed with `BadNotWritable`.

**Change:**
- `tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs`
- the common node replacements now emit `root.GetWriteMaskAsCode()` for
the `WriteMask` and `UserWriteMask` tokens instead of the hardcoded
`AttributeWriteMask.None`.
- `tools/Opc.Ua.SourceGeneration.Core/Schema/ModelDesignExtensions.cs` -
adds a `GetWriteMaskAsCode(this NodeDesign node)` helper that returns
`global::Opc.Ua.AttributeWriteMask.None` when `WriteAccess == 0`,
otherwise `(global::Opc.Ua.AttributeWriteMask){writeMask}`.

## Tests

-
`tests/Opc.Ua.SourceGeneration.Core.Tests/Schema/ModelDesignExtensionsTests.cs`
- adds `[TestCase]`-driven coverage for `GetWriteMaskAsCode` across
representative write-mask values plus a null-node test.

## Validation

- `dotnet build
tools\Opc.Ua.SourceGeneration.Core\Opc.Ua.SourceGeneration.Core.csproj
-c Release` - 0 warnings, 0 errors.
- `dotnet test
tests\Opc.Ua.SourceGeneration.Core.Tests\Opc.Ua.SourceGeneration.Core.Tests.csproj
-f net8.0 -c Release` - 3775 passed, 0 failed, 8 skipped.

## Note on the Views fix (excluded)

A second generator fix (preserve forward same-namespace `Organizes`
references to `ViewDesign` targets in `NodeSetToModelDesign`) was
intended to ship here as well, but it was excluded. That fix compensates
for behavior introduced by #4220 where imported Views are promoted to
top-level `ViewDesign` nodes and their `ParentNodeId` is cleared. On
`master` that promotion does not happen (Views remain children of their
folder), so the fix and its test do not apply and cannot be cleanly
isolated. It will land with #4220.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-14 07:57:44 -07:00
romanettandCopilot App cc4791e2fd Import NodeSet2 Views as standalone top-level nodes in the source generator (#4259) 
## Problem

The NodeState source generator mishandled `UAView` nodes imported from a
NodeSet2 model. When an exporter set a View's `ParentNodeId` to an
organizing folder, the importer/generator absorbed the View into that
folder's `Children` and tried to emit an invalid `AddChild(...)` call.
Because `ViewState` derives from `NodeState` (not `BaseInstanceState`),
a View can never be an `AddChild` component, so the generated code was
invalid.

## Fix

Make Views standalone top-level predefined nodes linked purely via
`Organizes` references — mirroring how `DataType`/`ReferenceType` type
nodes and `DataTypeEncoding` are handled:

-
**`tools/Opc.Ua.SourceGeneration.Core/Schema/NodeSetToModelDesign.cs`**
— clear the `UAView` `ParentNodeId` so the View stays a standalone
top-level node, keep the organizing folder's forward `Organizes`
references to those Views, and carry the View's `ContainsNoLoops`
attribute through to the `ViewDesign`.
-
**`tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs`**
— return `null` for `ViewDesign` in the `AddChild` and `ReplaceChild`
template paths so Views are never emitted as components.
-
**`tests/Opc.Ua.SourceGeneration.Core.Tests/Schema/NodeSetToModelDesignTests.cs`**
— new `ViewImport` NodeSet constant and
`ImportPreservesFolderOrganizesReferencesToViews` test verifying Views
are imported as top-level nodes while the organizing folder retains its
forward `Organizes` references to them.

## Validation

- `dotnet build tools\Opc.Ua.SourceGeneration.Core` (Release): 0
warnings / 0 errors.
- `dotnet test tests\Opc.Ua.SourceGeneration.Core.Tests` (net8.0,
filtered to `NodeSetToModelDesign`): all 17 tests pass, including the
new `ImportPreservesFolderOrganizesReferencesToViews`.

Extracted from the larger PR #4220 work — this PR contains only the
source-generator View fix (no WriteMask, UserAccessLevel, NodeSet2.xml,
or ReferenceNodeManager changes).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-14 07:57:06 -07:00
romanettandCopilot App 2c2bfa7080 Fix off-by-one ValueRank in the NodeState source generator (#4258) 
## Summary

`ModelDesignExtensions.GetValueRankAsCode` in the NodeState source
generator computed the ValueRank for a fixed-size multi-dimensional
`ArrayDimensions` string (e.g. `"3,3"`) as `dimensions.Length + 1`,
which over-counted the rank by one. The number of comma-separated
entries in `ArrayDimensions` already **is** the ValueRank — `"3,3"` is a
two-dimensional array with ValueRank `2`, not `3`.

## Changes

- `dims = dimensions.Length + 1` → `dims = dimensions.Length` (entry
count already equals the ValueRank).
- Added a guard: when there are zero entries (empty or all-commas
`ArrayDimensions`, which `Split(..., RemoveEmptyEntries)` reduces to
length 0), the method now returns `OneOrMoreDimensions`.
- Refreshed the stale explanatory comment / removed the obsolete `TODO`.
- Updated the `GetValueRankString_*` unit tests in
`ModelDesignExtensionsTests.cs` to the corrected expected ranks.

## Validation

- `dotnet build` of `Opc.Ua.SourceGeneration.Core` (Release): 0 warnings
/ 0 errors.
- Filtered `GetValueRankString` tests on net8.0: 18/18 passed.

Extracted from the larger PR #4220 work; this PR contains **only** the
ValueRank fix (two files).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
 2026-08-14 07:56:23 -07:00
Marc SchierandCopilot a7c776e8d4 Align the WoT implementation with spec: type binding, removed vocabulary, Alarms and Conditions (#4225) 
# Description

Aligns the WoT implementation with the normative changes in the OPC UA
WoT
specification drafts that the stack had not picked up, and fixes the
defects
that alignment surfaced.

The driving artifact is

[OPCF-Members/spec-drafts#19](https://github.com/OPCF-Members/spec-drafts/pull/19),
*"bind a projected node to an already-loaded type, by name or by
NodeId"*, which
landed on the spec `main` after this repository last tracked it.

## 1. Bind a projected node to an existing type (Binding §5.2.1)

§5.2.1 lets a document state that the node it projects is an instance of
a type
that **already exists** — one a companion model defines and the Server
has
loaded, or one a sibling document of the same conversion defines — so a
converter binds to that type instead of defining a second type of the
same
shape.

The converter hardcoded `HasTypeDefinition` to `BaseObjectType`, so a
document
that named its type projected an **untyped** node: a client browsing for
the
companion type would not find it, and nothing said why.

Both forms are now implemented, resolved against the §5.1.5 **local
context** —
the other documents being converted together first, a loaded
AddressSpace as the
fallback. `IWotNodeResolver` supplies that context (is a namespace held;
what
does a qualified BrowseName match; what does an ExpandedNodeId
identify),
`WotCompositeNodeResolver` composes implementations in the specified
order, and
`NullWotNodeResolver` is the default that holds nothing. The converter
needed no
new async surface: the binding is resolved in a pre-resolve pass,
exactly as
Thing references already are, and the synchronous core consumes the
result.

The §5.2.1 table is honoured: a unique name binds; the link settles an
ambiguous
name; a name that resolves to nothing while the link resolves is
**invalid**
(that is a mistake in the name, not a shorthand for the identifier); two
forms
resolving to different Nodes are invalid; a resolved type of the wrong
NodeClass
is invalid. A binding naming a type the local context does not hold
**fails**
rather than falling back to `BaseObjectType` — a silently mistyped node
is worse
than a reported failure.

A compact name is told from ordinary `@type` annotation **by
namespace**, not by
whether the lookup succeeds, so `saref:TemperatureSensor` on a Server
that has
never heard of SAREF stays the annotation it always was.

The definitive form — a `ua:HasTypeDefinition` link whose
`href` is the ExpandedNodeId of the type. An ExpandedNodeId matches
exactly one
Node or none, so it needs no lookup.

A Node has exactly one `HasTypeDefinition`, so:

- two such links → `AmbiguousTypeBinding` (6019), and the root keeps the
default
  rather than being bound to a guess;
- a link with no usable identifier → `InvalidTypeBinding` (6020);
- no binding at all → the `BaseObjectType` default is unchanged.

## 2. Drop the six vocabulary terms `#19` removed

`#19` deleted `uav:capability`, `uav:componentModel`, `uav:reference`,
`uav:congruentType`, `uav:congruentTypeName` and `uav:nameNamespace`
from the
context, the JSON Schema and the ontology. A link now names its
ReferenceType
directly in `rel`, so the three link relations become `ua:HasInterface`,
`ua:HasComponent` and `ua:NonHierarchicalReferences`, and the
congruent-type
pair is superseded by the §5.2.1 type binding.

Their validation and mapping are removed; they are now ordinary residue
like any
other unrecognised term.

### A real gap this uncovered

The converter knew **eight** ReferenceTypes by name and had neither
`NonHierarchicalReferences` (`i=32`) nor `HasInterface` (`i=17603`) — so
the two
relations that *replace* `uav:reference` and `uav:capability` could not
resolve
at all. That includes example 02, which this repository vendors as a
golden
asset and which `#19` rewrote to use them. Both are now in the map.

`uav:congruentType` was also the only term that redirected one reference
to
another while resolving a link target, so `ResolveTargetNodeIdAsync` no
longer
needs a loop. It resolves once and reports an unresolved target; the
resolution
context is still entered and the bytes still counted, so the
per-conversion
document, depth and byte bounds are unchanged.

## 3. Reject an out-of-range event severity instead of clamping it

`uav:severity` had grown in this implementation without being a
specification
term. Rather than drop it, it is proposed upstream in

[OPCF-Members/spec-drafts#21](https://github.com/OPCF-Members/spec-drafts/pull/21),
which defines it with an explicit rule: the value **shall** be in the
OPC 10000-5
range 1..1000, and a consumer **shall not** silently clamp it.

The implementation clamped, so a document asking for severity 5000 was
accepted
and published as 1000 — the mistake was hidden while what the author
asked for
was changed. An affordance carrying an out-of-range severity is now
skipped,
with the reason logged, and its half-built event type and
`GeneratesEvent`
reference are removed.

### Defect found while making that change

The occurrence-time path read `ValidateSeverity(severity) ??
tag.Severity`.
Because `ValidateSeverity(null)` returns the 500 default, it **never**
fell back
to the affordance's authored severity: an occurrence reported without a
severity
published 500 rather than the authored 900. Build-time validation and
the
occurrence-time choice are now separate methods.

## 4. Vendored examples and honest coverage claims

Examples 21 and 22 were missing from the vendored golden assets.
Converting
example 22 is now the end-to-end check for §5.2.1 — the specification's
own
example rather than a fixture written to match the implementation.

`docs/WotBindings.md` claimed ten of the eleven conformance units and
silently
omitted **WoT-ConditionMapping**, and said "twenty worked examples". The
count
is corrected, and WoT-ConditionMapping is now **covered** — see §5
below.

## 5. Alarms and Conditions (Binding §13)

The last unclaimed conformance unit. A projected Condition event now
derives
from the ConditionType it names rather than from `BaseEventType`;
falling back
would lose the Condition state model entirely, leaving a Client unable
to tell
an alarm from an ordinary event.

The two forms follow the hint-plus-pin pattern of §5.3:
`uav:conditionTypeId`
is definitive and wins, `uav:conditionType` is a readable hint resolved
for the
four ConditionTypes §13.1 scopes. An unpinned name outside that set is
reported
rather than guessed.

Four conformance rules are enforced, each because breaking it yields a
document
a consumer can read but cannot act on:

| Rule | Section | Diagnostic |
| --- | --- | --- |
| A Condition event declares `EventId` in its `data` | 13.3 |
`ConditionEventIdMissing` |
| `uav:conditionAction` is inside its closed set | 13.2 |
`InvalidConditionAction` |
| `uav:actsOn` names a Condition event in the same document | 13.4 |
`InvalidConditionTarget` |
| `Acknowledge`/`Confirm`/`AddComment` declare an `EventId` input | 13.4
| `ConditionActionInputMissing` |

`Enable` and `Disable` act on the Condition instance rather than one
occurrence
and are deliberately exempt from the last rule, pinned by a negative
test.
Shelving, suppression, dialog conditions and `ConditionRefresh` stay out
of
scope, as §13.1 scopes the mapping.

## 6. The sibling half of the §5.1.5 local context

`SnapshotWotNodeResolver` indexes the registry snapshot a conversion
runs over
and is wired into `WotNodeSetDocumentConverter`, so a document naming a
type
another document in the same registry projects resolves with no
AddressSpace at
all. Only Thing Models are indexed: a Thing Model projects a
`UAObjectType` and
is a valid target, a Thing Description projects an instance and never
is.

Identity comes from the new public
`WotNodeSetConverter.TryDescribeProjectedType`,
which applies exactly the rules the conversion uses, so an index entry
and the
projected node cannot drift apart. Ambiguity is preserved rather than
resolved:
two siblings sharing a qualified name yield both matches so the caller
can
report it.

## Deliberately not in this PR

- **The AddressSpace-backed `IWotNodeResolver`.** The abstraction, the
composite, the null default and the sibling-document implementation ship
here; the AddressSpace-backed half needs a cached type-hierarchy walk
behind
a server `OperationContext` and follows separately. Until it exists a
host
gets the sibling context plus the null fallback, which reports a binding
as
  unresolved rather than mistyping it.
- **Two §5.2.1 obligations that need the address space**: not
duplicating a
  mandatory instance declaration of the resolved type, and rejecting
  disagreement with an instantiated Thing Model.
- **Connectivity §7.3 parent resolution.**

Each is tracked; none is faked here.

## Related Issues

There is no tracking issue: this came out of a
spec-versus-implementation audit
rather than a reported defect, and the driving artifacts are the two
upstream
spec PRs linked above. Happy to open one if maintainers prefer — the
§5.2.1
follow-up in particular is worth an ADR, since the local-context
resolver is a
new injectable provider spanning two assemblies.

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

| | net10.0 | net48 |
|---|---|---|
| `Opc.Ua.Types.Tests` | 8573 / 8573 | 8555 / 8555 |
| `Opc.Ua.WotCon.Tests` | 1075 / 1075 | 1075 / 1075 |

Full `UA.slnx` build: **0 errors, 0 warnings** across all target
frameworks,
re-verified after `master` was merged into the branch.

Every new test was mutation-verified: the type-binding tests against
three
mutations (revert to the hardcoded default, drop the ambiguity check,
accept an
empty `href`), the severity tests against reinstating the clamp, and the
ReferenceType additions against removing them from the map. Each
mutation failed
the suite before the change was kept.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
 2026-08-14 07:55:33 -07:00
Marc SchierandCopilot 64d2834b2f Confine capture artifact paths to the configured base folder (#4247) 
## What

`CaptureSessionManager.StartAsync` validates the caller-supplied
`SessionFolder`
against the configured capture base folder, then copies `PcapFilePath`
and
`KeyLogFilePath` from the same request through **without validating
them**.

Both reach the filesystem:

| Property | Sink | Effect |
| --- | --- | --- |
| `PcapFilePath` (write) | `PcapFileWriter.OpenCurrentFile` —
`FileMode.Create`, after `EnsureParentDirectoryExists` | truncates and
overwrites the named file, creating parent directories |
| `PcapFilePath` (read) | `ReplayCaptureSource` →
`PcapFormatter.FormatAsync` → `File.ReadAllBytesAsync` | returns the
named file's bytes to the caller |
| `KeyLogFilePath` | `OpenOrCreate` + seek to end | appends key material
to the named file |

`InProcessCaptureSource.ResolveArtifactPath` returns a rooted path
unchanged and
`Path.Combine`s a relative one without normalizing, so `..` traversal is
not
contained either.

Where those values arrive from outside the process — the MCP
`start_capture`
tool passes a `StartCaptureRequest` straight through — a caller can name
any
path the process can open, in either direction.

## Why this looks like an oversight

The containment already exists and is applied consistently everywhere
else in
this package. `PacketDecodeTools.DecodePcapWithKeysAsync` and
`PacketReplayTools.ReplayPcapAsync` both run caller paths through
`McpCapturePath.ResolveAndValidate(path, allowedRoot)` before use.
`start_capture`
is the one entry point that skips it.

## Change

Adds `ValidateAndResolveArtifactPath` beside the existing
`ValidateAndResolveSessionFolder` and applies it to both artifact
properties.
Relative paths are resolved against the validated session folder,
preserving
the `StartCaptureRequest` contract, and the result is then confined to
the
configured capture base folder. Containment comparisons are
case-insensitive on
Windows and case-sensitive on non-Windows platforms. Null and empty
paths pass
through untouched so sources keep deriving their default names inside
the
session folder.

## Tests

Five added to `SessionFolderValidationTests`:

- rejects a rooted `PcapFilePath` outside the base folder
- rejects `..` traversal in `PcapFilePath`
- rejects a rooted `KeyLogFilePath` outside the base folder
- accepts artifact paths inside the base folder and resolves relative
paths beneath the session folder
- rejects a case-variant sibling path on Linux

Each was verified to **fail without the change**. MCP capture, decode,
and
replay fixtures now place valid session artifacts beneath the configured
capture root while preserving explicit outside-root rejection coverage.
`SessionFolderValidationTests` (11/11) and the complete
`Opc.Ua.Tools.Tests` suite (430/430) pass locally.

## Note

This has security relevance for hosts that expose the MCP diagnostics
tools, so
it was also reported to `securityteam@opcfoundation.org` per
`SECURITY.md`.
Happy to close this and handle it privately instead if you would prefer.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b8ab3763-b9bd-46ae-b0ef-045db26e5782
 2026-08-13 08:20:33 +00:00
Marc SchierandCopilot fac30c45e6 Add MinSupportedSamplingInterval configuration and honor it when revising sampling intervals (#4243) 
# Description

PR #4194 removed the unused
`ServerConfiguration.MinMetadataSamplingInterval` and left nothing in
its place. As a result there was no way to populate
`Server.ServerCapabilities.MinSupportedSampleRate` (`i=2272`) —
`ServerInternalData.CreateServerObjectAsync` hardcoded it to `0`.
Separately, the only clamp applied when revising a monitored item's
sampling interval was the per-node
`BaseVariableState.MinimumSamplingInterval`, which defaults to
`MinimumSamplingIntervals.Continuous` (`0`), so reference-server nodes
accepted any requested interval (e.g. 10 ms) unrevised — exactly what
was reported.

This adds a configuration entry **in the exact schema slot #4194
vacated** and enforces it as a lower bound.

## Configuration

- **`ServerConfiguration.MinSupportedSamplingInterval`** — `double`
(milliseconds), default `0`, `[DataTypeField(Order = 17)]`, i.e. between
`MaxNotificationsPerPublish` (16) and `AvailableSamplingRates` (18).
- `ApplicationConfiguration.xsd` — `<xs:element
name="MinSupportedSamplingInterval" type="xs:double" minOccurs="0" />`
in the same position the removed element occupied.
- Fluent API **`SetMinSupportedSamplingInterval(double)`** on
`IApplicationConfigurationBuilderServerOptions`, in the place
`SetMinMetadataSamplingInterval` was removed from.
- The element is added to the six config XML files #4194 touched
(reference / CTT / AOT / GDS use `0`; `test-server-full.xml` uses a
distinctive non-zero value for the round-trip test).

The address-space Property keeps its OPC UA spec name, so
`Server.ServerCapabilities.MinSupportedSampleRate` and
`VariableIds.Server_ServerCapabilities_MinSupportedSampleRate` are
unchanged.

```xml
<MaxNotificationsPerPublish>1000</MaxNotificationsPerPublish>
<MinSupportedSamplingInterval>2000</MinSupportedSamplingInterval>
<AvailableSamplingRates>
```

```csharp
application.Build(applicationUri, productUri)
    .AsServer([endpointUrl])
    .SetMinSupportedSamplingInterval(2000);
```

## Behavior

- `ServerInternalData` now publishes the configured value to
`Server.ServerCapabilities.MinSupportedSampleRate`.
- New `SubscriptionManager.CalculateRevisedSamplingInterval(...)` (two
overloads, alongside the existing `CalculateRevisedQueueSize`)
centralizes the revision rule and replaces the logic that was duplicated
across four call sites — the create and modify paths of
`AsyncCustomNodeManager` and `CustomNodeManager` — which now expose a
`MinSupportedSamplingInterval` property seeded from the configuration
(mirroring how `MaxQueueSize` is initialized).

The rule:

1. A requested interval below zero resolves to the *default sampling
interval*: the **publishing interval of the subscription** on create,
and the item's **current sampling interval** on modify. The modify case
preserves 1.5.378 behavior, so a `ModifyMonitoredItems` call that leaves
the sampling interval unspecified does not silently retune the item.
2. A node declaring `MinimumSamplingIntervals.Continuous` (`0`) for the
`Value` Attribute reports **by exception** and is **not** bound by the
server-wide minimum — the requested interval is returned unchanged.
3. Every other item is raised to `max(nodeMinimumSamplingInterval,
MinSupportedSamplingInterval)`. Attributes other than `Value`, nodes
declaring `MinimumSamplingIntervals.Indeterminate` (`-1`), and
non-Variable nodes are bound only by `MinSupportedSamplingInterval`.
4. `double.MaxValue` is capped to one year (pre-existing behavior, now
in one place).

Event monitored items do not sample and are unaffected.

**Backward compatible:** with the default of `0` the computation is
identical to the previous code for every input, including `Continuous`,
`Indeterminate`, positive node minimums, non-`Value` attributes and
`NaN`. No existing configuration or deployment changes behavior.

Node managers that implement `INodeManager` directly can call the new
public helper to apply the same rule.

## Related Issues

- Fixes #4191

The issue thread also asked whether
`Subscription.MaxNotificationsPerPublish` is revised by the server. It
is (`SubscriptionManager.CalculateMaxNotificationsPerPublish`), but it
was untested and is not observable from the `CreateSubscription`
response, which carries no revised field for that parameter. Four tests
were added that assert it through
`SubscriptionDiagnostics.MaxNotificationsPerPublish` on a privileged
session, covering create and modify, the above-limit, below-limit and
`0` (server default) cases.

## Tests

- **`SamplingIntervalRevisionTests`** (17 unit tests,
`Opc.Ua.Server.Tests`) — the full revision matrix: negative request
resolving to the default interval, the `Continuous` bypass,
`Indeterminate`, node minimum above *and* below the floor, a floor of
`0` being a no-op, non-`Value` attributes, non-Variable and null nodes,
and the `double.MaxValue` cap.
- **`MinSupportedSamplingIntervalTests`** (10 integration tests,
`Opc.Ua.Subscriptions.Tests`) — against a reference server configured
with a 500 ms floor: the capabilities node reflects the configuration; a
node declaring 100 ms is lifted to 500 ms;
`Scalar_Static_Arrays2D_LocalizedText` (1000 ms) wins over the floor;
`CurrentTime` (250 ms) is lifted; a `Continuous` node keeps its 10 ms; a
non-`Value` attribute is floored; `ModifyMonitoredItems` behaves the
same in both directions; event items are unaffected.
- Restored the configuration builder and round-trip test coverage that
#4194 deleted.
- `TestFixture` gained a `protected virtual void
ConfigureServer(ApplicationConfiguration)` hook so a derived fixture can
vary the server configuration before start. Default implementation is a
no-op, so no existing fixture changes behavior.

## Documentation

- `docs/Subscriptions.md` — new **Sampling interval revision** section
with the input table, the four-step rule (including both
negative-interval fallbacks), XML and fluent configuration examples, and
guidance for custom node managers.
- `docs/migrate/2.0.x/configuration.md` — new section documenting that
`MinMetadataSamplingInterval` was removed and
`MinSupportedSamplingInterval` added. Verified against `master378`: the
old setting defaulted to `1000` but was never read, and
`MinSupportedSampleRate` was hardcoded to `0` with no server-wide floor.
The guide therefore stresses that these are **two different settings,
not a rename** — consumers should drop the old element (a default of `0`
reproduces 1.5.378 exactly) and only opt into the new one deliberately,
since reusing the old value would silently introduce a sampling floor.

## Validation

- `dotnet build UA.slnx` — **0 warnings, 0 errors** across all TFMs.
- .NET 10: `Opc.Ua.Server.Tests` 4091 passed,
`Opc.Ua.Subscriptions.Tests` 619 passed (full suite including the new
fixture), `Opc.Ua.InformationModel.Tests` 1063 passed,
`Opc.Ua.Configuration.Tests` 221 passed, `Opc.Ua.Gds.Tests` 119 passed.
- .NET Framework 4.8: new unit tests (200 incl. sampling groups), new
integration tests (17) and configuration round-trip (135) all passed.
- `ConsoleReferenceServer` starts successfully with the updated
configuration file.

## 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>
Copilot-Session: 79f8090c-b2a3-409c-a5bf-56b6d2a35e70
 2026-08-12 17:41:29 +00:00
Marc Schier a9a3e021ee Fix VirtualFileSystem compiler crashes under memory pressure (#4246) 
# Description

Replace `VirtualFileSystem`'s per-file delayed-commit memory mapping
with a
growable managed buffer made of 64 KB chunks. The mapping was private
and
non-shared, but reserved 64 MB on 32-bit processes or 256 MB on 64-bit
processes for every virtual file. Under commit pressure, touching a
lazily
allocated page could raise an uncatchable `AccessViolationException`
inside the
source generator and terminate the compiler.

The new implementation:

- allocates memory in 64 KB chunks as content grows;
- preserves the existing synchronous stream contract and adds direct
span and
  asynchronous stream paths;
- zero-fills sparse gaps and discarded content when files shrink and
regrow;
- supports empty physical files, preserves their last-write timestamps,
and
  prevents read-stream seeks from extending file length;
- seals `VirtualFileSystem`, which was not intended as a supported
subclassing
  API.

## Related Issues

- Fixes #4233

## Validation

- `dotnet test tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -f
net10.0 --filter "Category=FileSystem"`
- `dotnet test tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -f
net48 --filter "Category=FileSystem"`
- `dotnet test
tests\Opc.Ua.SourceGeneration.Core.Tests\Opc.Ua.SourceGeneration.Core.Tests.csproj
-f net10.0`
- Scoped whitespace, style, and analyzer formatting checks for the
changed
  source and test files

## Checklist

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

---------

Copilot-Session: 38223d6a-b707-4255-940f-9e67f666590c
 2026-08-12 14:33:35 +00:00
Marc SchierandCopilot 6795073d2d Mount OpenUSD representations with HasAddIn (#4239) 
# Description

Mount every `OpenUsdRepresentation` AddIn with `HasAddIn` rather than
plain
`HasComponent`.

- Correct the `IOpenUsdRepresentedType` placeholder in the OpenUSD
NodeSet.
- Route Pump, Minimal Robot, and Intent sample authoring through the
shared
  `CreateRepresentation` helper.
- Remove redundant representation subtree rebasing from sample call
sites.
- Add model, in-process sample, and over-the-wire regression coverage.
- Update the OpenUSD documentation and sample diagrams.

`HasAddIn` is a subtype of `HasComponent`, so the incorrect reference
still
browsed and rendered successfully but failed companion-model
conformance.

## Related Issues

No related issue.

## Validation

- `Opc.Ua.OpenUsd.Tests`: 914 passed on .NET 10.
- Pump and Minimal Robot sample builds completed without warnings.
- Pump and Robot `RepresentationsAreMountedWithHasAddInAsync` E2E tests
passed.

## Checklist

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 19119d71-e1bc-41c9-ba0d-2d1dea437167
 2026-08-12 11:31:55 +02:00
Marc Schier 80db326567 Fix MCP diagnostics tool contracts and capture failures (#4241) 
# Description

Fix the MCP packet-diagnostics tool contract and the capture failures
reported in #4238.

- Accept and emit canonical hyphenated capture source and format names
while preserving CLR-name and integer compatibility.
- Restore typed enum constraints in generated MCP schemas and surface
actionable diagnostics errors to MCP clients.
- Stop `list_interfaces` from reading `LinkType` on unopened SharpPcap
devices.
- Close failed SharpPcap handles before retrying NIC opens and report
permission failures immediately.
- Normalize capture roots, remove duplicated path segments, and use a
lazy secure per-user or atomic temporary fallback.
- Align MCP/diagnostics documentation and interop-agent guidance.

## Related Issues

- Fixes #4238

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

---------

Copilot-Session: 641a4356-aa8d-45e6-9077-eeb50b9b7e0a
 2026-08-12 10:33:56 +02:00
Marc SchierandCopilot 1cca744b5c Retire the last exposed locks: node, browser, server diagnostics and application properties (#4222) 
Closes #4201. Closes #4202. Closes #4203. Closes #4215. Closes #4216.

(#4215 and #4216 are @marcschier's originals from the #4188 review;
#4202 and #4201 are my later duplicates of the same two findings. All
four describe the two changes in section 1 and are closed together.)

**This PR now carries what was #4221 as well.** The two were a stack;
they have been unstacked and combined here, and #4221 is closed.
Everything below the "Read the server diagnostics" section is the work
that was previously reviewed as #4221.

---

## 1. The browse path guards itself, and the last node locks are gone

`AsyncCustomNodeManager` kept two `lock (source)` sites around
`CreateBrowser`, carrying a NOTE that browser creation was not
internally synchronized. `NodeBrowser` still handed its lock to derived
types through `protected Lock DataLock`.

`NodeState.CreateBrowser` now holds a browse lock across
`PopulateBrowser` and `OnPopulateBrowser`, so browser construction on a
node is serialized — which is what the external `lock (source)` was
actually providing, and what a handler that mutates the node during
population (the historian's lazy config installer) depends on. Both
`lock (source)` statements and the NOTE are gone.

**What that lock does not do**, stated plainly in the XML docs and in
`docs/NodeManagers.md` rather than over-promised: it is not an atomic
snapshot across the node's children, notifiers and references. Writers
take those collections' own locks, not the browse lock. The browser is a
point-in-time copy and each collection is read consistently, but the
combination is not a transaction.

No `lock` takes a `NodeState` as its sync root any more. That was the
last thing keeping `NodeState` from being documentably self-guarding, so
the other eleven went with it: `lock (activeNode)`, `lock (target)`,
`lock (source.Node)`, `lock (target.Node)`, `lock (notifier)`, `lock
(handle.Node)` in `AsyncCustomNodeManager`, and `lock (node)` in
`CustomNodeManager2`. Most were a check-then-act pair now served by
`NodeState.AddReferenceIfMissing`, which does the check and the insert
under the node's own lock.

`NodeBrowser.DataLock` is removed. A browser is **single-consumer**: it
belongs to whoever created it, and both server browse paths already
serialize on the owning side. The exposed lock only added an
inheritance-level locking contract that a derived browser could not
reason about.

## 2. Read the server diagnostics under the lock every writer takes

`OnUpdateDiagnostics` locked the diagnostics **payload** rather than the
lock every writer takes through `UpdateServerDiagnostics`, so the
snapshot a client reads from the `ServerDiagnostics` node was never
excluded from a concurrent writer.

Both halves matter, and the second is the one that is easy to miss:

```csharp
lock (m_diagnosticsLock)                                        // was: lock (ServerDiagnostics)
{
    value = Variant.FromStructure(ServerDiagnostics, copy: true);  // was: copy defaulted to false
}
```

`copy: true` is what makes it a snapshot. `FromStructure(value, copy:
false)` does `new ExtensionObject(value, copy)` — it wraps the **live**
structure without copying it, so the caller would read the fields long
after the lock was released and see them change underneath. Swapping the
lock alone would have looked like a fix while still excluding nothing.

## 3. Analyzer numbering

The browser rule is **UA0027**, not UA0025. This branch had
independently taken UA0025 for `NodeBrowser.DataLock` while #4188
published UA0025 for `ILocalNode.DataLock` and UA0026 for
`BaseVariableValue.Lock`. Master's are shipped, so they keep the
numbers:

| | UA0025 | UA0026 | UA0027 |
|---|---|---|---|
| master (#4188) | `ILocalNode.DataLock` | `BaseVariableValue.Lock` | —
|
| **this PR** | `ILocalNode.DataLock` | `BaseVariableValue.Lock` |
**`NodeBrowser.DataLock`** |

Both sides had written a different `UA0025Tests.cs`, which arrived as an
add/add conflict. They are now split: `UA0025Tests.cs` holds master's
`ILocalNode` tests, `UA0027Tests.cs` holds the `NodeBrowser` ones.

UA0024's help text also drops its reference to `ReadServerDiagnostics`,
which #4193 removed as unused, and names `ISession.ReadDiagnostics` /
`ISubscription.ReadDiagnostics` instead — those do still exist.


## 4. Two more locks reachable from outside their owner

**`ApplicationConfiguration.PropertiesLock` is removed.** It returned
the properties dictionary itself, so `lock
(configuration.PropertiesLock)` was `lock (configuration.Properties)` —
the lock *was* the data it guarded, the same shape as #4203.

It had exactly one production consumer
(`OAuth2CredentialCollection.Load`, a get-or-add with `ParseExtension`
called inside the lock), so it could not simply be deleted. `Properties`
is now a `ConcurrentDictionary` — same public type, every operation
atomic on its own — and the one multi-operation need gets a member of
its own:

```csharp
public T GetOrAddProperty<T>(string key, Func<T> valueFactory);
```

It deliberately does **not** invoke the factory under a lock, so
`ParseExtension` now runs outside any critical section. Holding a lock
across a caller-supplied callback is exactly the shape that produced the
attribute/diagnostics deadlock earlier in this work.

The two existing tests asserted only `PropertiesLock is not null`, so
they tested nothing but its presence. They are replaced by tests of the
contract: factory runs once for a missing key, not at all for a present
one, 32 concurrent callers all observe the same instance, null arguments
throw.

`PropertiesLock` ships in 1.5.378, so this is a breaking removal —
migration guide section plus analyzer **UA0028**.

**`SharedDefinitionCache` no longer hands out its lock.** Three sites in
`SharedKeyValueSubscriptionStore` took the nested cache's `Lock` to
operate on the cache's own dictionary. Both are private now, behind
`ReplaceAll` and `CloneWhere`.

`SnapshotCommitLock` stays exposed on purpose: it is a `SemaphoreSlim`
sequencing an async commit across `await` points, which
`System.Threading.Lock` cannot span, so folding it in would change
behaviour rather than hide a lock.
## Verification

Merged `master` (`0edfe237a`, including #4234) with no conflicts.

- `dotnet build UA.slnx` — 0 warnings, 0 errors
- `Opc.Ua.Server.Tests` — 4074 passed
- `Opc.Ua.Types.Tests` — 8501 passed
- `Opc.Ua.MigrationAnalyzer.Tests` — 151 passed on all five TFMs (138 →
151, the new ones being UA0027 and UA0028)
- `Opc.Ua.Core.Tests` — 4249 passed
- `Opc.Ua.Redundancy.Server.Tests` — 573 passed

## Note for the reviewer

@romanett — your approval was given when this PR's diff was the 7
diagnostics files, because it was measured against
`marcschier/node-browse-locks`. Now that the two are combined and the
base is `master`, the diff is 23 files, and the extra 16 are the
node/browser lock work that was #4221. GitHub kept the approval across
the retarget, but it did not cover that code, so please re-review rather
than let it ride.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041
Copilot-Session: b3c0e146-5b9e-4d08-a0f6-db3f940484b1
Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63
Copilot-Session: 0ee22c1d-c319-4257-bf78-a5a077a1824a
 2026-08-11 22:14:10 +02:00
Marc SchierCopilotCopilot Autofix powered by AI
 		466c5cb982 Robot intent follow-ups: multi-controller executors, spec fixes, and an MCP tool library (#4195) 
Follow-ups to #4165, plus the work that followed from re-checking this
branch against the specification. Everything started as a defect found
by review or by probing the running code; the specification is unchanged
except where noted.

The TLS certificate lifetime fix (#4196) and the OpenUSD work (#4198)
were both merged into this branch. #4196 has since landed on master, so
it no longer appears in this diff. #4198 was closed in favour of this PR
and its content is here.

The MCP trim/AOT work (#4228), the `DataGenerator` null `NodeId` fix
(#4227) and the NodeSet external-parent import fix (#4229) have since
been merged in as well. All three are closed in favour of this PR and
their content is here, described under *Making the MCP libraries trim
and AOT clean*, *A generator that returned the value you opted out of*
and *A NodeSet parent that lived outside the import batch* below.

## A two-arm cell could not be built

`IntentExecution` carried no controller identity, and there was
effectively one `IIntentExecutor` per server. An executor handed an
execution therefore could not tell **which arm** it was for, so a cell
with two `IntentControllerType` instances — an ordinary configuration —
was not expressible through the builder or DI.

Both halves are addressed:

- `IntentExecution` now carries `ControllerId` (`NodeId`, the OPC UA
identity) and `ControllerName`, so one shared executor can discriminate;
- DI gained per-controller registration —
`AddRobotIntentExecutor<TExecutor>(controllerBrowseName)` and an
instance overload — so two genuinely different executors can coexist.

Resolution order is `WithExecutor` → per-controller DI registration →
the single DI executor → a build-time throw. There is still **no silent
fallback** to a rejecting executor; that was a Critical defect fixed in
#4165 and it stays fixed. `IIntentExecutor` is unchanged, so this is
purely additive.

## Declarations that were quietly untrue

- **`Accepts<T>` silently discarded a redeclaration.** Declaring an
intent type twice — say to narrow `cancelSupported` on the second call —
kept the first, wider declaration and said nothing. That is the same
class of defect as the capability-honesty problems #4165 fixed
(`PauseSupported` declaring a pause the host could not perform;
`Aborting` degrading silently to `Buffered`). Identical redeclarations
remain idempotent; conflicting ones now throw `BadInvalidArgument`.
- **`RI-Interop-40010` assumed Annex B item 3 rather than checking it.**
It now requires exact set equality between the OPC 40010
`TaskProgramName` values and the Robot Intent `ProgramType.ProgramId`
values. Both sides are readable from the address space without
commanding the robot, which is what makes the requirement structural
under §12.1.

## Two things the specification requires that this did not do

Re-checking the implementation against the merged draft revision turned
up two genuine violations. Both are fixed, and the NodeSet is untouched
by either.

- **Every graphed mission reported `Failed` on success.**
`MissionRules.SelectTransition` returned `null` for two different
situations — a graph *terminus*, which is the normal success path, and a
branch point whose conditions all evaluated false — and the caller
collapsed both into `Failed`/`NoTransition`. A three-way result
(`Terminus` / `Selected` / `NoConditionMatched`) separates them, so a
mission that runs to its end now completes. The failure reason surfaces
through the inherited Part 10 result container rather than a new member.
- **Facet URIs were never published.** An upstream `may` became a
`shall` in the revision this branch adopts.
`RobotIntentConformanceUris.Facets` held 31 URIs with **zero**
production references — the set existed but nothing emitted it. The
published set is now derived from `RobotIntentFacetCalculator`, the same
authority that already computed `SupportedFacets`, rather than from a
second hand-maintained list that could drift; a reflection test pins the
two together.

## An easy client mistake

`RequestAuthorityAsync` returns a lease the caller must remember to
inspect. Forgetting makes every later submission fail with
`ControlNotOwned` for reasons that are invisible at the call site.
`RequireAuthorityAsync` throws instead, and names the current owner.
`RequestAuthorityAsync` is untouched — inspecting and branching is
legitimate.

## Letting an agent drive the robot

New `tools/Opc.Ua.Mcp.Robotics` gives a language model the intent API as
MCP tools: 36 across discovery, monitoring, direct control and missions,
behind a `Robotics` tool profile, composable into any MCP host exactly
as the existing OPC UA tool packages are. The `IntentViewerClient`
sample hosts them behind `--mcp` (stdio by default, HTTP when `--view`,
since MCP stdio and the in-process viewer would otherwise share stdout),
and the sample gained a pallet scene so an agent can pick and stack
parts and watch the result in the OpenUSD viewer. All the scene logic is
server-side — no viewer or connector changes.

Four gaps in the client API had to be filled first, since the tools are
a thin layer over it and could not have been written honestly without
them: `ReadStateAsync`, `ListOperationsAsync`/`ListMissionsAsync`,
controller-level
`CancelIntent`/`CancelAll`/`Pause`/`Resume`/`Retry`/`ReleaseControl`,
and `WaitForCompletionAsync`.

The tools refuse rather than paper over: a refusal from the server is
returned with its `IntentFailureEnum` and never retried, and command
authority is never acquired as a side effect of a tool call.

**One defect this surfaced:** `--profile robotics` shipped 36 tools that
could not do anything. Every robotics tool resolves a named OPC UA
session and only the connection tools can open one, so with no `Connect`
in the catalogue the whole profile failed. A test had pinned the defect
in place, asserting the profile did *not* contain `Connect` on the line
below the `diagnostics` assertion requiring it. Each package now
declares its own requirement in its registration extension rather than
the host special-casing it, so an application embedding
`Opc.Ua.Mcp.Robotics` directly gets a usable catalogue without
reproducing the host's pairing. `Full` still takes the connection tools
from the core package, so they are never registered twice. The
replacement test states the invariant — a profile exposing
session-scoped tools must expose the tools that open a session — rather
than the instance.

## Making the MCP libraries trim and AOT clean

*Merged from #4228.*

`Opc.Ua.Mcp.Core` is now `IsAotCompatible` on `net10.0`, and the
reflection the analysers then reported is gone. Turning the analysers on
is the point: until now they were not running on these libraries at all
— including on the tool library this branch adds.

- **`OpcUaJsonHelper.Serialize`** went through `JsonSerializer` (IL2026
/ IL3050). The conversion helpers in that class already reduce every OPC
UA value to a closed set — `null`, the primitive scalars, `string`,
`Dictionary<string, object?>` and lists thereof — so the JSON is now
written directly with a `Utf8JsonWriter`.

Output is unchanged, and that is asserted rather than assumed:
`OpcUaJsonHelperTests.SerializeMatchesTheReflectionBasedSerializer`
compares the new writer byte for byte against `JsonSerializer` for every
shape the helpers emit, including all numeric widths, the
custom-formatted scalars, nested sequences and strings that force the
encoder to escape.

One deliberate contract change: arbitrary object graphs are no longer
accepted. Serializing them needs reflection, so `Serialize` now throws
`NotSupportedException` naming the type instead of silently stringifying
it. No call site in the repository passes anything outside the closed
set.

- **`McpSchemaFilters`** used `JsonSerializer.SerializeToElement` on a
`JsonObject`; it now parses the node's own text, which needs no
reflection.

`Opc.Ua.Mcp.Diagnostics` is deliberately **not** marked. It depends on
SharpPcap and on the reflective service-call dissection in
`Opc.Ua.Core.Diagnostics`, neither of which is trim safe — which is why
that package does not opt in either. Two improvements are kept
regardless: `PacketDecodeTools` serializes `DecodedServiceCall` through
a source-generated `JsonSerializerContext`, and
`PacketCaptureTools.ListInterfaces` now carries the
`RequiresUnreferencedCode` / `RequiresDynamicCode` annotations that
`NetworkInterfaceEnumerator.ListLocalInterfaces` already declares, so
the constraint is visible to callers instead of implicit. Nothing is
suppressed.

Marking an assembly `IsAotCompatible` only proves its own IL is clean;
it says nothing about whether the library still functions once
reflection is gone. So `Opc.Ua.Aot.Tests` references `Opc.Ua.Mcp.Core`
and exercises it with JSON reflection disabled, stating what is and is
not true today:

- tool **results** serialize without reflection — this would fail
outright if the `Utf8JsonWriter` change regressed;
- tool **registration** still requires reflection, because the MCP SDK
builds each tool's input schema by asking `JsonSchemaExporter` for a
`JsonTypeInfo` of every parameter type. Without a source-generated
context covering the tool signatures — and with the injected
`OpcUaSessionManager` being described rather than treated as a service —
that throws `NotSupportedException`.

The second test asserts the throw. It documents the remaining gap and
turns red the moment someone closes it, rather than leaving it to be
found in a published binary. The package README says the same thing in
prose.

**Two pre-existing issues this surfaced, both outside this PR:** pulling
`Opc.Ua.Mcp.Diagnostics` into an AOT publish reports `IL2046` / `IL3051`
on `NicCaptureSource.StartAsync`, which is annotated
`RequiresUnreferencedCode` / `RequiresDynamicCode` while the
`ICaptureSource.StartAsync` it implements is not, and `IL2075` on
`ServiceCallReassembler.GetCollectionCount` / `GetPropertyString`, which
call `Type.GetProperty` on the result of `GetType()`. Happy to file
these separately if useful.

## A generator that returned the value you opted out of

*Merged from #4227. Fixes #4226.*

`GetRandomNodeId(useBoundaryValues: false)` could still return
`NodeId.Null` — one of the boundary values the same method offers when
boundary values *are* requested. A caller that opted out should not
receive one, and `GetRandomExpandedNodeId` builds on it and inherited
the same behaviour.

The `IdType.Opaque` branch is the practical path:
`GetRandomByteString()` draws its length with
`NextInt32(MaxStringLength)`, which is inclusive of zero, and a
zero-length opaque identifier in namespace 0 satisfies `NodeId.IsNull`.
With three namespaces and `MaxStringLength` at 10 that is roughly **one
call in 130**. A numeric identifier of `0` in namespace 0 is null too,
at around one in four billion.

The draw is now repeated while the result is null and boundary values
were not requested. Guarding the result rather than the individual
branches covers every identifier type, including the numeric case,
rather than only the one that happens to be reachable today. The
`useBoundaryValues: true` path is untouched, so boundary values remain
available when they are asked for.

Four tests in `tests/Opc.Ua.Core.Tests/Types/DataGeneratorTests.cs` loop
20,000 draws, because at one call in 130 a single draw proves nothing.
Against the **unmodified** generator they fail on draw 93 and draw 1346.
Two of the four exist to keep the fix honest rather than to test the
bug: `GetRandomNodeIdStillProducesEveryIdentifierType` catches a "fix"
that simply stopped emitting opaque identifiers, which would satisfy the
null assertions while silently narrowing the generator, and
`GetRandomNodeIdWithBoundaryValuesStillYieldsThem` confirms the leak was
closed rather than the feature removed.

**How it was found:** an intermittent downstream test failure. A test
server initialising a static `NodeId`-valued variable via
`TestDataSystem.ReadValue` occasionally stored `NodeId.Null`; the
encoder correctly writes that as JSON `null`, and a consumer asserting a
value was present failed about one run in five — never reproducibly in
isolation. It only surfaced under load because `UnsecureRandom.Shared`
is a fixed-seed, process-global generator shared between `DataGenerator`
and OPC UA transport jitter, so the number of draws preceding
address-space initialisation depends on what else is running in the
process.

## A NodeSet parent that lived outside the import batch

*Merged from #4229.*

`UANodeSetHelpers.LinkParentChildRelationships` builds its lookup table
from **only the nodes in the current import batch**, links a child when
the parent is found — and then cleared `instance.Handle`
**unconditionally**, which is exactly where `Import` had stashed the
authored `ParentNodeId`. A node whose parent lives in another NodeSet,
or is owned by another NodeManager, therefore lost that parent
**silently** at import, and nothing downstream could recover it.
`RuntimeNodeSetNodeManager.CreateAddressSpaceAsync` already has a step
that wires references to external node managers; the parent link never
reached it.

Such a parent is now recorded rather than discarded, and exposed through
`UANodeSet.TryGetUnresolvedParentNodeId`. A parent that resolved
normally inside the batch is **not** recorded, so a caller does not
re-wire references that already exist. The record lives in a
`ConditionalWeakTable`, so it does not keep an imported node alive and
does not widen `NodeState` — whose `Handle` is a general-purpose slot a
caller may legitimately use for its own purposes, and so is not a safe
place to leave the value.

`RuntimeNodeSetNodeManager` consumes it to report the one case that
cannot be repaired automatically. In NodeSet2 the `ParentNodeId`
attribute is a *hint*; the authoritative edge is the entry under
`<References>`, which the reverse-reference pass already turns into an
external reference. A node that names an out-of-manager parent but
carries **no such Reference** materializes with no path from that
parent, and no ReferenceType can be inferred to synthesise one — so it
is reported as an authoring defect rather than guessed at or silently
accepted.

This is deliberately **not** a fix for a specific address-space symptom.
It removes a silent loss of authored information and gives callers the
means to act on it. Whether any particular orphaned-node report is
caused by this path, or by namespace-index translation elsewhere, still
needs separate diagnosis.

**Changed while merging.** As authored in #4229 the accessor was
`GetUnresolvedParentNodeId`, returning `NodeId?`. `NodeId` is a readonly
struct implementing `INullable`, so that was a `System.Nullable<NodeId>`
wrapped around a type that already carries its own null representation —
which this repository excludes in favour of an `IsNull` check, and it
asks for `TryGet` over casting on struct types. The tests
correspondingly asserted `Is.Null` against a struct, which the testing
rules also exclude. It is now `bool
TryGetUnresolvedParentNodeId(NodeState, out NodeId)`, yielding
`NodeId.Null` when there is no unresolved parent. The API is unshipped —
it arrives with this branch — so nothing depended on the previous shape.
Both tests were re-verified by mutation after the rewrite: disabling the
record in `LinkParentChildRelationships` still fails
`ImportKeepsAParentThatIsNotInTheSameBatch`.

## Dependencies

- **OpenUSD to `0.7.0-alpha`.** All 11 issues we filed upstream are
fixed, so the workarounds they justified are gone and the parts of the
pump and generator samples that were blocked on them are implemented.
- **ModelContextProtocol to `2.1.0`** across all five MCP tool libraries
and the executable. No code changes were needed — the surface we use is
unchanged in 2.x — but central package management required a cascade of
pins, each commented where it sits. Both transports were verified
against a running server rather than assumed. MCP 2.x is stateless (no
`initialize`, no `Mcp-Session-Id`); an OPC UA session is application
state addressed by a caller-supplied name, which fits that model but
does assume process affinity, and that assumption is now written down
rather than implied.

## Infrastructure

The `code coverage` gate was measuring nothing on this branch and
reporting success. It is fixed and now enforced, which immediately
exposed real gaps — most usefully `RoboticsIntentJson`, which parses
agent-supplied JSON (untrusted input from a language model) and had no
tests at all. It is now at full line coverage in CI. Patch coverage is
**90.20%** against a 75% floor.

The `netstandard2.1` build leg was also failing. The MCP projects build
as empty shells under a legacy `CustomTestTarget`, and the viewer sample
referenced them on the target framework alone, so it tried to resolve a
net10.0 shell from net8.0. The sample now derives MCP support from
whether those projects are actually being built.

## From review

A review pass over the branch found two real defects, both fixed.
`RoboticsIntentJson` parses JSON written by a language model, so every
malformed shape it can be handed is reachable input; it indexed pose
arrays without checking their length and enumerated properties without
checking their kind, so a short or wrongly-typed array escaped as
`IndexOutOfRangeException` or `InvalidOperationException` rather than
the argument error the tool descriptions promise the agent. Separately,
`IntentOperationHandle` treated a zero `QueuePosition` as "not observed"
and discarded it, so a handle kept reporting the last queued position
after the server had said the operation left the queue.

The pose validation immediately caught a latent test defect: the pallet
scenario interpolated doubles into JSON with the current culture, so
under a comma-decimal culture `[0.12, 0.16, 0.28]` became a six-element
array. It passed only because that submission is expected to be refused
for want of authority, so the wrong coordinates were never used.

A third review finding — that a default-constructed
`RobotIntentControllerState` hands out a null `NodeId` — was checked and
rejected: `NodeId`, `QualifiedName` and `LocalizedText` are readonly
structs here, so `default` is the null value and `IsNull` is `true`. The
tests asserting that invariant are kept.

A second review pass over the merged branch found two more, both fixed
and both about an error surfacing as the wrong kind of failure rather
than about a wrong result:

- **Malformed agent JSON did not arrive as an argument error.**
`RoboticsIntentJson` turns every *validation* failure into an
`ArgumentException` — a wrong root kind, a missing property, a short
pose array — but three `JsonDocument.Parse` calls and the NodeId parse
were unguarded. A syntax error escaped as `JsonException` and an
unparseable NodeId as `ServiceResultException`, which are the two shapes
a language model is most likely to produce. That is the same defect
class as the pose/kind checking above: a caller separating bad input
from a genuine fault had to know to catch three unrelated exception
types. Parsing now goes through one helper that reports a syntax error
as an `ArgumentException` naming the argument, keeping the
`JsonException` as the inner exception so the position is not lost, and
the NodeId goes through `NodeId.TryParse` rather than `Parse`,
collapsing its two failure modes into one.
`MalformedIntentJsonIsRejected` had asserted
`InstanceOf<JsonException>`, pinning the behaviour rather than the
intended contract; it now asserts `ArgumentException` like every
neighbouring test, and three tests cover the newly guarded paths.
- **`ConditionalWeakTable.Add` throws on a duplicate key.** Not
reachable today, but `TryGetUnresolvedParentNodeId` documents `Handle`
as a general-purpose slot a caller may use for its own purposes — and a
caller doing exactly that before re-importing into the same collection
would take a hard throw from inside the import pipeline. Removing first
makes the most recent import win. `AddOrUpdate` would say this more
directly but does not exist on `netstandard2.0`.

## Deliberately not fixed here

`FastenIntentDataType.Joint` is refused when the Server exposes no OPC
40450/40451 model, and a client cannot discover that before calling.
That is **not fixable in this repository**: §5.4.2 makes `Joint` the
defining mechanism while §11.3 says the parameters otherwise stand
alone, and `IntentCapabilityDataType` declares capabilities at
intent-type granularity with no member-level mechanism anywhere in the
model. It is a specification gap, raised as S4 on
[marcschier/opcua-drafts#60](https://github.com/marcschier/opcua-drafts/pull/60)
rather than worked around with a bespoke mechanism the spec does not
define.

## Validation

`dotnet build UA.slnx -c Release` on the merged branch — **0 errors, 0
warnings**, with the AOT analysers now running over `Opc.Ua.Mcp.Core`.

| Suite (net10.0) | Tests |
|---|---|
| `Opc.Ua.Core.Tests` | 4247 (86 skipped) |
| `Opc.Ua.Robotics.Tests` | 540 |
| `Opc.Ua.OpenUsd.Tests` | 909 |
| `Opc.Ua.Tools.Tests` | 419 |
| `Opc.Ua.Server.Tests` — `RuntimeNodeSet` | 94 |
| `Opc.Ua.Robotics.Intent.Tests` | 22 |
| `Opc.Ua.Aot.Tests` — `McpAotTests` | 3 |

0 failures. `Opc.Ua.Tools.Tests` rose from 409 to 419 with the
serializer-equivalence tests from #4228 and the malformed-JSON tests
from the second review pass, and `Opc.Ua.Core.Tests` covers the
`DataGenerator` tests from #4227 and the import tests from #4229.
`UANodeSetHelpers` also passes on **net48** (5/5). The MCP
agent-usability grader scores Core 100 / Full 100.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
Copilot-Session: a0718e8a-a158-45c5-929d-e1d6c0982431
 2026-08-11 13:03:36 +02:00
Alexander KöpkeMarc SchierMarc SchierCopilot
 		0edfe237a6 Bound in-memory historian retention (#4234) 
# Description

Bounds raw-data memory usage in `InMemoryHistorianProvider` with a
configurable source-timestamp retention window.

- Adds `RawDataRetentionPeriod`, defaulting to one hour; `TimeSpan.Zero`
restores unbounded retention.
- Keeps `MaxSamplesPerNode` as an additional hard cap, with both limits
enforced.
- Applies retention across bulk, best-effort, update-as-insert, and
transactional write paths.
- Refreshes the retained high-water timestamp after raw deletions so
later out-of-order inserts use the current archive contents.
- Explicitly keeps the ReferenceServer conformance historian unbounded
because it seeds a multi-hour archive and tests arbitrary historical
timestamps.
- Documents the default and configuration in `HistoricalAccess.md` and
the pump sample README.
- Adds regression coverage for defaults, custom and unbounded windows,
combined limits, bulk/atomic writes, out-of-order inserts, and both raw
deletion APIs.

## Related Issues

Addresses the multi-gigabyte memory growth observed when
`PumpDeviceIntegrationServer` runs continuously with its in-memory
historian enabled.

## Checklist

- [x] I have signed the CLA and read the CONTRIBUTING guide.
- [x] I have added tests that prove the fix is effective.
- [x] I have added the necessary documentation.
- [x] I have verified the targeted build and analyzer-enabled historian
test suite complete without warnings.
- [ ] I ran all tests locally using `UA.slnx` against .NET Framework and
.NET 10.
- [x] I have addressed all PR feedback received.

---------

Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@hotmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-08-11 08:14:10 +02:00
Marc SchierandCopilot 5bc8a9d705 Reduce ISession: type the continuation points, delete the obsolete validate overload (#4199) 
## Stacked on #4197

Base is `marcschier/server-bind-phase`. The full stack is **#4188 →
#4193 → #4197 → this**.
GitHub retargets each base automatically as the stack merges. **Review
the earlier three
first.**

**#4217 has been merged into this branch** (squash `53dd4b618`), so the
last remaining
item from the issue — `ISession.SessionDiagnostics` — is now part of
this PR.

Closes #4187.

Refs #4203 — **does not close it.** #4217 swapped `OnUpdateDiagnostics`
onto
`m_diagnosticsLock`, but it still calls
`Variant.FromStructure(ServerDiagnostics)` with the
default `copy: false`, which wraps the *live* structure in the
`ExtensionObject`. The caller
reads its fields after the lock is released, so the lock currently
excludes nothing. #4222
adds `copy: true` and is what actually closes #4203.

## The finding that changed the approach

`ISession` has **exactly one production adapter**:
`src/Opc.Ua.Server/Session/Session.cs`.
The other `ISession` implementations in the tree are the unrelated
**client** interface
(`src/Opc.Ua.Client/Session/Session.cs`, `RedundantClientSession`) and a
migration-analyzer test stub.

One adapter means the seam is hypothetical; two means it is real. The
issue proposed
splitting `ISession` into five concern-interfaces — that would turn one
hypothetical seam
into five. The interface earns its keep for testability and because node
managers receive
it through `OperationContext`, but that argues for making it
**smaller**, not for
multiplying it.

**So: reduce, don't split.** Reasoning posted to the issue as
[comment
5226712936](https://github.com/OPCFoundation/UA-.NETStandard/issues/4187#issuecomment-5226712936)
so the five-way framing does not get re-proposed later.

## Measurement

38 members, 368 references. External production consumers are **4 files
using 5 members**:

| Consumer | Members used |
|---|---|
| `Opc.Ua.Redundancy.Server/Sessions/DistributedSessionManager.cs` |
`Id`, `InitializeAsync`, `Dispose` |
| `Opc.Ua.Di.Server/Locking/DefaultLockService.cs` | `Id` |
| `samples/ConsoleReferenceServer/UAServer.cs` | `Id`, `Identity`,
`ReadDiagnostics` |
| `samples/Quickstarts.Servers/.../DataChangeMonitoredItem.cs` | type
reference only |

Every production caller of the five continuation-point members is inside
`Opc.Ua.Server`
(`MasterNodeManager`, `NodeManagerLifecycle`, `HistorianDispatcher`).

**`ISession` is 38 → 32 members.**

## Commits

### `882b2937` Type the history continuation point

`ISession` leaked `object` twice, against the repository rule:

```csharp
void SaveHistoryContinuationPoint(Guid id, object continuationPoint);
object? RestoreHistoryContinuationPoint(ByteString continuationPoint);
```

The erasure was **not** hiding a genuinely opaque plugin token.
`HistorianDispatcher` is the
only production caller, it always passes a `HistorianContinuationState`,
and it casts
straight back to that type on restore. The `object` was a round trip
through the type system
that bought nothing.

`HistorianContinuationState` is `internal`, so it cannot appear on a
public interface.
`IHistoryContinuationPoint` names the concept instead: it carries an
`Id` and extends
`IDisposable`.

Two latent bugs fell out of typing it:

- The session disposed only those points that **happened** to implement
`IDisposable` and
**silently leaked the rest**. Every point is now disposable by
construction, and three
  `as IDisposable` probes are gone.
- Carrying the `Id` on the point removes the separate `Guid` parameter,
which could disagree
with the payload it was filed under. The mock stores in the historian
tests had the same
  latent inconsistency.

### `0d020b77` Delete the obsolete synchronous `ValidateBeforeActivate`

**Zero production callers** — the nine references were all tests written
for it. `[Obsolete]`
since 1.5.378, which the repository rules allow removing in master.

It is also the divergent-signature problem the runtime analysis flagged:
the async form
returns the validated handler and policy as a tuple, while the sync one
took the same two
values as trailing `out` parameters, so a caller had to learn two shapes
for one operation.

The overload could not verify a user token requiring decryption, so on a
secure endpoint it
failed closed and told the caller to use the async path anyway. The one
test asserting that
fail-closed behaviour goes with the method it described; the path it
pointed at is already
covered by `ActivationValidationDecodesBinaryEncodedUserIdentityToken`.

The other eight tests are **converted, not deleted** — the channel,
signature,
security-policy and token-policy validation they cover is still
asserted, now against the
path the server actually uses.

### `821e8302` Route the continuation points behind one member

Five `ISession` members were one-line pass-throughs to
`SessionContinuationPoints`, a
419-line module that already owns the behaviour and already has its own
`IContinuationPointStore` seam. The interface restated the module's
surface without adding
anything.

```csharp
// was
session.SaveContinuationPoint(cp);
session.RestoreHistoryContinuationPoint(bytes);

// now
session.ContinuationPoints.SaveBrowse(cp);
session.ContinuationPoints.RestoreHistory(bytes);
```

The holder is `internal sealed`, so `ISessionContinuationPoints` is what
makes it
addressable.

**This is composition, not the service locator `IServerContext` refused
to be.** A subsystem
with an independent lifetime should be injected; continuation points
belong to the session
and cannot exist without it.

`InvalidateContinuationPoints` called `RemoveBrowseForManager` and
`RemoveHistoryForManager`
in sequence, so the pair becomes `RemoveForManager` — the caller's
intent is that a node
manager is going away, not that two collections need visiting.

## Tests

Three harnesses had hand-rolled a `Dictionary<Guid, object>` to stand in
for the holder. They
now use the **real** one, so the dispatcher and `BrowseNext` meet the
eviction and disposal
rules a live session actually applies.

**One hazard worth recording for future interface work.** A Moq member
that is never stubbed
returns `null`. Where the old code called a method straight on
`ISession`, that `null` was
harmless. Reaching *through* a property means an unstubbed
`ContinuationPoints` null-derefs
instead. Three tests failed exactly that way; they are wired to a real
holder, not restubbed.

| Suite | net10.0 | net48 |
|---|---|---|
| `Opc.Ua.Server.Tests` | 4032 passed, 1 failed | builds clean |
| Touched areas (continuation points, historian, master node manager,
session) | 898 passed | — |

The single failure is
`ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a
local certificate-store problem verified to reproduce unchanged at HEAD
with these changes
stashed. Not caused by this PR.

`Opc.Ua.Redundancy.Server`, `Opc.Ua.Di.Server`, `Quickstarts.Servers`
and
`ConsoleReferenceServer` all build clean.

## Docs

`docs/MigrationGuide.md` gains a "Removed members on ISession" section
covering the
`ValidateBeforeActivate` removal with the async replacement, and the
`IHistoryContinuationPoint` change with before/after.

## The SessionDiagnostics removal (folded in from #4217)

`ISession.SessionDiagnostics` handed out the whole mutable
`SessionDiagnosticsDataType` —
the structure the session's diagnostics lock protects — so a caller
could read a field while
the owner was writing it. Every server-side reader wanted exactly one
value out of it, so the
interface now exposes those two values (`SessionName`,
`ClientApplicationUri`) directly and
the structure stays behind the lock.

That work landed here as #4217 rather than as a separate PR, and it also
fixes #4203:
`OnUpdateDiagnostics` now takes `m_diagnosticsLock` instead of locking
the diagnostics
payload it was handed.

## Reviewer questions

- `ISessionContinuationPoints` exposes `MaxBrowse` as read-only while
the concrete holder
keeps a setter (the test framework adjusts it to exercise limits).
Should the setter be on
the interface, or is test-only mutation better left to the concrete
type?
- `RemoveForManager` collapses two removal methods. The concrete
`RemoveBrowseForManager`/`RemoveHistoryForManager` remain public on the
holder — worth
  making them private now that nothing outside calls them individually?

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041
 2026-08-11 06:57:40 +02:00
Marc SchierandCopilot 9375a2431b Retire the IServerInternal bind mutators and freeze the bind phase (#4197) 
## Stacked on #4193

Base is `marcschier/server-context`, which is itself based on #4188.
**Review #4188 and
#4193 first.** GitHub retargets each base automatically as the stack
merges.

Closes #4184.

## What this does

`IServerInternal` published twelve `Set*` binding methods plus
`CreateServerObjectAsync`,
so any holder of the interface could rewire a running server. This takes
them off the
interface and makes a late bind fail loudly.

## The audit changed the shape of the work

The plan this came from said "design a frozen bind phase". **One already
exists.**
`StandardServer` has a seventeen-method `Create*` factory seam, and
`DependencyInjectionStandardServer` overrides seven of those to resolve
from the container:

```csharp
protected override IRoleManager CreateRoleManager(IServerInternal server, ApplicationConfiguration configuration)
    => m_services.GetService<IRoleManager>() ?? base.CreateRoleManager(server, configuration);
```

`StandardServer` then carries each factory result into the datastore:

```csharp
ServerInternal.SetRoleManager(CreateRoleManager(m_serverInternal, configuration));
```

So an integrator already had two supported routes — override the
virtual, or register in
DI. `Set*` on the interface was a **third route that bypassed the
ordering**, and the only
one that let a caller rebind a running server.

The job was therefore to stop publishing plumbing, not to build a phase.

## Measurement

Every call site of the twelve mutators plus `CreateServerObjectAsync`:

| Area | Count | Where |
|---|---:|---|
| `Opc.Ua.Server` | **11** | `StandardServer.cs:3729–3844`, one
contiguous block |
| samples | **1** | `ReferenceServer.cs:219`, inside a
`CreateMasterNodeManager` override |
| tests | 20 | `ServerInternalDataTests` (18), `SubscriptionTests` (2) —
**all on the concrete `ServerInternalData`** |

`SetIdentityRegistry` had **zero** call sites anywhere.

The earlier estimate for this work was 268 files. That number counted
files *referencing*
`IServerInternal`, which this change never touches: the interface
survives and only the
mutators move off it.

## Commits

### `e719a88a` Add the missing `CreateUserManagement` factory seam

The one gap, and the reason this had to land first. User management was
the only subsystem
with **no factory seam** — the only way to populate
`IServerInternal.UserManagement` was
`SetUserManagement`, which is what `ReferenceServer` called from inside
a
`CreateMasterNodeManager` override. Removing the mutator without a
replacement would have
broken that scenario with no migration path.

The DI path at `OpcUaServerBuilderExtensions.cs:1178` builds a
`UserNamePasswordAuthenticator` from a registered `IUserManagement` — a
different thing
entirely that never reaches the Part 18 §5 model on the address space.

Purely additive. `ReferenceServer` moves onto the override.

The test covers the **override** seam rather than the DI seam
deliberately: registering an
`IUserManagement` in the container also switches on the
username/password authenticator, so
a DI-based test exercises two unrelated paths and fails for reasons
unrelated to binding.

### `cbcfda47` Remove `SetIdentityRegistry`

Zero call sites, despite a doc comment advertising it as the integrator
seam. The supported
route has always been
`ServerIdentityRegistryExtensions.RegisterDefaultAuthenticators`,
which *adds* authenticators to the default registry rather than
replacing it.
`IdentityRegistry` is read five times outside `Opc.Ua.Server`, so the
default is what every
consumer already uses.

**The analyzer confirmed the reading**: with the setter gone, `RCS1170`
fired on the
property — it is provably never reassigned after construction, so it is
now read-only.

### `ea0937a2` Take the bind mutators off `IServerInternal`

The mutators stay `public` on `ServerInternalData`, which owns the
state. Only the interface
loses them. The eleven call sites switch from the `ServerInternal`
property to the
`m_serverInternal` field, already typed `ServerInternalData?` — the same
one-token change
`GetStatus` and `EnterState` took in #4193.

**No test changed.** All twenty test call sites already used the
concrete type, which is the
clearest evidence these were never interface-level API.

`CS1574` found six doc comments across `IRoleManager`, `RoleManager`,
`IUserManagement`,
`UserManagement`, `IServerInternal` and `SessionManager` that pointed
integrators at the
mutators. They now point at the factory seam.

### `f3c4b9d3` Refuse to bind after the server has started

Each bind goes through a guard; `StandardServer` closes the phase once
the session manager
is in place. A late `Set*` throws
`ServiceResultException(BadInvalidState)` **naming the
operation** and pointing at the factory seam — otherwise the failure is
indistinguishable
from a configuration problem.

Rebinding *during* startup stays legal, which is what let both audit
hazards resolve with no
behaviour change:

- **`SetNodeManager` also binds three more managers**
(`DiagnosticsNodeManager`,
`ConfigurationNodeManager`, `CoreNodeManager`). Intentional — they are
reached through the
master node manager and have no independent lifetime — and now
documented rather than
  silent.
- **`SetSessionManager` unhooks the previous manager's handlers**, which
reads as
written-for-repeat-calls. Nothing rebinds it, but the freeze does not
make the path dead
either, since binding twice during startup is still allowed. Left as-is;
removing it is a
  separate question.

## Tests

Four new tests on the freeze: the refusal itself across four
representative mutators, that
the message names the operation, that rebinding stays legal until the
phase closes, and that
closing twice is harmless. Plus one hosted-server test for the
`CreateUserManagement` seam.

| Suite | net10.0 | net48 |
|---|---|---|
| `Opc.Ua.Server.Tests` | 4033 passed, 1 failed | 230 passed, 1 failed
(filtered) |
| `Opc.Ua.Redundancy.Server.Tests` | 573 passed | — |
| `Opc.Ua.Redundancy.Kubernetes.Tests` | 97 passed | n/a (net8/net10
only) |

The single failure is
`ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a
local certificate-store problem verified to reproduce unchanged at HEAD
with these changes
stashed. Not caused by this PR.

## Docs

`docs/MigrationGuide.md` gains a table mapping every removed mutator to
the `Create*`
override and the DI registration that replace it, plus the
`SetIdentityRegistry` removal
with its rationale.

## Net effect across #4193 and this PR

`IServerInternal` goes from ~460 lines to **~200**. It no longer
publishes locks, dead
members, or the ability to rewire a running server.

## Reviewer questions

- `CompleteBindPhase()` is public on `ServerInternalData` but
deliberately **not** on
`IServerInternal` — only the server owning the startup sequence should
decide when binding
  is over. Reasonable, or should it be `internal`?
- `SetSessionManager`'s unhook path is now unreachable in practice but
still legal during
startup. Leave it, or remove it and assert single-bind for that one
subsystem?

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041
Copilot-Session: b3c0e146-5b9e-4d08-a0f6-db3f940484b1
Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63
 2026-08-11 00:27:45 +02:00
Marc SchierandCopilot 4e0b82af5f Add IServerContext and move server startup tasks onto it (#4193) 
## Stacked on #4188

Base is `marcschier/server-seam-analysis`, not `master`.
`IServerInternal.cs` and
`ServerInternalData.cs` both diverge from master on that branch (chunk
4c removed the two
`object` locks), so branching off master would conflict on both files
*and* design
`IServerContext` against a stale interface. GitHub retargets the base to
`master`
automatically when #4188 merges. **Review #4188 first.**

Closes part of #4184.

## What this does

`IServerInternal` is a service locator: 56 members, 30-odd of which hand
out one of 20+
subsystems. A component that only needs to observe the server and act on
it has to take the
whole thing, and no signature tells a reader which parts a component
actually touches.

This adds `IServerContext` — the ambient view of a running server — and
moves
`IServerStartupTask` onto it.

```csharp
public interface IServerContext
{
    ServerState CurrentState { get; }
    ServerObjectState ServerObject { get; }
    IServiceMessageContext MessageContext { get; }
    ServerSystemContext DefaultSystemContext { get; }

    ServerSystemContext CreateSystemContext(ISession session);
    T? FindPredefinedNode<T>(NodeId nodeId) where T : NodeState;
    IEnumerable<T> FindNodeManagers<T>() where T : class;

    ValueTask ReportEventAsync(IFilterTarget e, CancellationToken ct = default);
    ValueTask CloseSessionAsync(OperationContext context, NodeId sessionId,
        bool deleteSubscriptions, CancellationToken ct = default);
    ValueTask DeleteSubscriptionAsync(uint subscriptionId, CancellationToken ct = default);

    void UpdateServerDiagnostics(Action<ServerDiagnosticsSummaryDataType> update);
}
```

Nothing on it returns a subsystem. A component that needs one takes it
as a constructor
dependency, which states the requirement instead of hiding it behind an
ambient handle.

`IServerInternal : IServerContext`, so every existing handle already
satisfies the new seam
and consumers migrate one at a time.

## The member set was measured, not guessed

Every member of `IServerInternal` was counted by read site across `src`,
`samples` and
`tests`, split by area. `Ext` is non-test production consumers, which is
what decides
whether something belongs on an ambient interface.

| Member | InServer | OtherSrc | Sample | Test | **Ext** | On
`ISystemContext`? |
|---|---:|---:|---:|---:|---:|---|
| `NamespaceUris` | 63 | 50 | 34 | 131 | **84** | yes |
| `Telemetry` | 58 | 23 | 35 | 5 | **58** | yes |
| `MessageContext` | 2 | 8 | 7 | 11 | **15** | no |
| `TypeTree` | 42 | 3 | 11 | 10 | **14** | yes, as `TypeTable` |
| `NodeManager` | 66 | 7 | 6 | 74 | **13** | no |
| `DefaultSystemContext` | 21 | 4 | 1 | 7 | **5** | no |
| `IdentityRegistry` | 1 | 4 | 1 | 1 | **5** | no |
| `SubscriptionManager` | 23 | 0 | 0 | 17 | **0** | no |
| `SessionManager` | 21 | 0 | 0 | 15 | **0** | no |
| `RequestManager` | 10 | 0 | 0 | 23 | **0** | no |
| `AggregateManager` | 12 | 0 | 0 | 2 | **0** | no |
| `ModellingRulesManager`, `ConformanceUnitsManager`,
`DiagnosticsEnabled` | 0 | 0 | 0 | 0 | **0** | no |

Two things fall out. The highest-traffic members are already carried by
`ISystemContext`,
which `ServerSystemContext` copies off the server at construction — so
most of those 278
`NamespaceUris` reads are callers reaching through the server for
something they already
hold. And the subsystems with zero external demand are exactly the ones
that should be
injected rather than published.

The check that matters: the eight `IServerStartupTask` implementations
in
`Opc.Ua.Redundancy.Server` are the heaviest real consumer of the ambient
handle, and
between them they touch **eight distinct members**. That is the shape
the interface is cut
to.

## Commits

### `e4a912eb` Remove seven unused members from `IServerInternal`

The deletion test, applied before anything is designed against the
interface.

| Removed | Evidence |
|---|---|
| `CloseSession(OperationContext, NodeId, bool)` | zero call sites;
sync-over-async; already obsolete |
| `ReadServerDiagnostics<TResult>` | zero production sites — **added one
commit earlier in #4188 for symmetry and never called** |
| `ModellingRulesManager` | property never read; `ServerInternalData`
owns the lifetime |
| `ConformanceUnitsManager` | same |
| `DiagnosticsEnabled` | façade over
`IDiagnosticsNodeManager.DiagnosticsEnabled`, which is what callers use
|
| `Status` | one site, itself inside an obsolete method |
| `ServerDiagnostics` | only reachable through the diagnostics methods
that replaced it |

`EndpointAddresses` was kept — `Opc.Ua.Di.Tests` uses it to resolve an
endpoint URL. Thin,
but real.

The only build fallout was three lines in one test assembly.
`Opc.Ua.Redundancy.Server`,
`Opc.Ua.Redundancy.Kubernetes`, `Opc.Ua.Gds.Server.Common`,
`Opc.Ua.WotCon.Server`,
`Opc.Ua.PubSub.Server`, `Opc.Ua.Di.Server`, `ConsoleReferenceServer` and
`Quickstarts.Servers` all built untouched. That is the evidence the
members were dead.

### `ffcc80b9` Add `IServerContext`

Additive. `ServerInternalData` implements it; no caller changes.

### `e8c4e15f` Hand startup tasks `IServerContext`

Nine implementations, `OpcUaServerHostedService`, and the test call
sites.

### `1b4de97a` Drop the `IServerStartupTask` migration-guide section

Review feedback. `IServerStartupTask` does not exist in `master378`, so
it is a 2.0-only
concept and nobody migrating from 1.5.378 can own an implementation of
it. The **Removed
members on `IServerInternal`** section stays - those members *are* in
master378 - and is now
self-contained.

### `2ab7d689` Merge `marcschier/server-seam-analysis`

Picks up the base branch including its merge of master. Textually clean,
no conflicts.

### `6a650f12` Migrate `RobotIntentHostStartupTask` to `IServerContext`

The merge was textually clean but **semantically broken**: master's
Robot Intent feature
(#4165) added a tenth `IServerStartupTask` implementation written
against the old
`IServerInternal` signature, so it no longer satisfied the interface.
Its body walked
`server.NodeManager.AsyncNodeManagers` and type-tested each entry for
`RobotIntentNodeManager` - exactly the pattern `FindNodeManagers<T>()`
replaces - so it
migrated the same way as the other nine. `FindNodeManagers` also
searches the synchronous
registry and de-dupes by reference, so a manager registered on both
faces still starts its
intent controller hosts exactly once.

## Three findings from migrating real consumers

**`MessageContext` is not derivable.** The design had callers use
`DefaultSystemContext.AsMessageContext()`, on the reasoning that a
message context can be
built from a system context. It can't: that conversion carries the
namespace tables and the
factory but **not** `MaxStringLength`, `MaxArrayLength` or
`MaxByteStringLength`. Four
replication paths in `Opc.Ua.Redundancy.Server` would have silently
fallen back to default
decoding limits — a component quietly accepting more than the server is
configured to. It
is a real member, and the interface documents why.

**`new`-shadowed members split the mock surface.** `IServerInternal`
first redeclared
`DefaultSystemContext` with a narrower type and `CurrentState` with a
setter. That
compiles, but a shadowed member is a *second interface slot*: production
code calling
through `IServerContext` read a slot that every existing
`Mock<IServerInternal>` leaves
unstubbed. Seventeen tests went from passing to `NullReferenceException`
**while still
compiling**. The narrower typing bought no encapsulation anyway — the
object really is a
`ServerSystemContext` and any caller can cast — so both shadows are
gone. `CurrentState` is
read-only on the ambient interface and the one site that assigns it uses
the concrete type,
as `GetStatus` already does.

**`IServiceProvider` is deliberately *not* on `OnServerStartedAsync`.**
The plan sketched
it. All nine implementations already receive their dependencies through
their constructors
from the builder extensions, so a container on the callback would have
replaced one service
locator with another.

## `FindNodeManagers<T>`

Two tasks walked `server.NodeManager.NodeManagers` and type-tested each
entry for
`ILocalAddressSpaceSource`; two more reached for a specific manager to
cast it to a
capability. Asking for the capability says what the caller wants; asking
for the registry
says how to find it.

```csharp
// before
foreach (INodeManager nodeManager in server.NodeManager.NodeManagers)
{
    if (nodeManager is not ILocalAddressSpaceSource source) { continue; }
    ...
}

// after
foreach (ILocalAddressSpaceSource source in server.FindNodeManagers<ILocalAddressSpaceSource>())
{
    ...
}
```

It searches both the asynchronous and synchronous registries:
`ToSyncNodeManager()` returns
`this` when a manager already implements `INodeManager`, so the two
lists overlap and a
capability may sit on either face.

## Tests

Mock harnesses are **wired, not stubbed** — they return the node
managers and contexts the
tests already build, so the tests keep asserting against real behaviour
rather than against
a mock that returns whatever is convenient. Four new tests cover
`CreateSystemContext`
(identity/locale propagation, null guard), the explicit
`DefaultSystemContext` identity,
and `FindPredefinedNode` before the address space is bound.

| Suite | net10.0 | net48 |
|---|---|---|
| `Opc.Ua.Redundancy.Server.Tests` | 573 passed | 573 passed |
| `Opc.Ua.Redundancy.Kubernetes.Tests` | 97 passed | n/a (net8/net10
only) |
| `Opc.Ua.Server.Tests` | 4028 passed, 1 failed | 225 passed, 1 failed
(filtered) |

The single failure is
`ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`,
a local certificate-store problem. Verified it reproduces unchanged at
HEAD with these
changes stashed — it is not caused by this PR.

All twelve dependent assemblies and samples build clean on both TFMs,
zero warnings.

### Re-verified after the base merge (`6a650f12`)

`Opc.Ua.Server`, `Redundancy.Server`, `Redundancy.Kubernetes`,
`Robotics.Server`,
`WotCon.Server`, `Di.Server`, `PubSub.Server`, `Gds.Server.Common` and
`XRegistry.Server` all build clean on net10.0. `Opc.Ua.Robotics.Tests`
491 passed,
`Opc.Ua.Redundancy.Server.Tests` 573 passed,
`Opc.Ua.Redundancy.Kubernetes.Tests` 97 passed.

### CI status: 119 passed, 3 failed - none caused by this PR

| Failing job | Why it is not this PR |
|---|---|
| `SourceGeneration_Core_Tests_windows` (net48) |
`GenerateAndCompileDesignFileAsync(..., CSharp11, ...)` fails with
`CS9058: Feature 'Lock object' is not available in C# 11.0` because
generated `Opc.Ua.NodeStates.g.cs` emits `System.Threading.Lock`.
**Identical failure on base #4188 (build 16737)**, which contains none
of these commits. This PR touches no source-generation file. |
| `SourceGeneration_Core_Tests_windows` (net10.0) | Same root cause;
also failing on base #4188. |
| `PubSub_Eth_Tests_windows` (net48) | Agent DNS outage:
`NuGetToolInstaller` died with `ERR:getaddrinfo ENOTFOUND
dist.nuget.org`. The job aborted before restore/build, so **the test
step never executed**. Infra flake; needs a re-run, not a code change. |

Both `SourceGeneration.Core` failures belong to the base branch and
should be fixed there
rather than papered over here.

## Correction to the recorded plan

Chunk 7 (the frozen bind phase) was recorded as the largest mechanical
footprint remaining
and the one most likely to stall. It isn't: **each of the 12 `Set*`
mutators has exactly one
call site**, in the `StandardServer` wiring, plus one sample. It is a
~13 call-site change.
The 268-file figure that produced that assessment counted files
*referencing*
`IServerInternal`, which chunk 7 never has to touch, because the
interface survives and only
the mutators move off it.

## Docs

`docs/MigrationGuide.md` gains one section: the removed
`IServerInternal` members with their
replacements, plus the note that `MessageContext`,
`DefaultSystemContext`, `CurrentState`,
`ServerObject`, `ReportEventAsync`, `CloseSessionAsync`,
`DeleteSubscriptionAsync` and
`UpdateServerDiagnostics` moved *down* to `IServerContext` and stay
reachable through
`IServerInternal` unchanged, so no call site has to move.

An earlier revision also documented the `IServerStartupTask` signature
change there. That
was dropped after review (1b4de97a4): `IServerStartupTask` does not
exist in `master378`, so
it is a 2.0-only concept and nobody migrating from 1.5.378 can own an
implementation of it.
The rationale for the ambient view - including the `AsMessageContext()`
decoding-limit trap -
lives on the `IServerContext` and `IServerStartupTask` XML docs instead,
which is where a
2.0-only concept belongs.

## Reviewer questions

- `IServerContext` is 11 members, not the 9 the design targeted.
`MessageContext` and
`FindNodeManagers<T>` were added because migrating real consumers showed
the alternatives
were wrong (silent decoding-limit loss) or a pass-through
(`NodeManager.NodeManagers` plus
  a type test at every call site). Reasonable?
- `IServerInternal : IServerContext` keeps the fat interface alive on
purpose — it is what
makes this a signature change rather than a 268-file rewrite. It only
shrinks once the DI
migration lands. The 6a deletions are unconditional, so even a full
stall leaves it seven
  members smaller.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041
Copilot-Session: b3c0e146-5b9e-4d08-a0f6-db3f940484b1
Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63
 2026-08-10 21:14:09 +02:00
Marc SchierandCopilot 4cbcec6e23 Update NuGet dependencies to latest and version ASP.NET Core per target framework (#4223) 
# Description

Refreshes the centrally-managed NuGet dependencies in
`Directory.Packages.props` to their latest stable versions, and changes
how the ASP.NET Core packages are pinned so their version follows the
target framework.

## ASP.NET Core packages are now versioned per TFM

`Microsoft.AspNetCore.Authentication.Certificate`,
`Microsoft.AspNetCore.Authentication.JwtBearer`,
`Microsoft.AspNetCore.Mvc.Testing` and `Microsoft.AspNetCore.TestHost`
ship one band per .NET major, carry no `netstandard2.0` asset, and do
not roll forward across majors — a `net8.0` project simply cannot
consume the `10.0.x` band. A single central pin therefore had to sit on
the oldest supported band and hold *every* consumer there, which is why
these sat at `8.0.28` while `10.0.10` was current.

They are now selected from `$(TargetFramework)`, mirroring the existing
MQTTnet split:

| TFM | Version |
|---|---|
| `net8.0` | `8.0.29` |
| `net9.0` | `9.0.18` |
| everything else | `10.0.10` |

The last group is the fallback rather than an explicit `net10.0` match,
so a TFM the file does not enumerate — a future `net11.0`, or the
`net10.0` shell that legacy `netstandard2.0`/`netstandard2.1`
`$(CustomTestTarget)` builds fall back to — still resolves a version.
Verified against the generated `project.assets.json` that each TFM of
`Opc.Ua.Bindings.Https` really does resolve its own band.

## Version changes

`Crdt`/`Crdt.Transport` and the `RaftCs` family 1.1.2 · `Dekaf` 1.11.2 ·
`JsonSchema.Net` 9.4.0 · `Microsoft.Bcl.TimeProvider`, the
`Microsoft.Extensions.*` set and
`System.Composition.AttributedModel`/`System.Formats.Asn1`/`System.Text.Json`/`System.Threading.Channels`/`System.Threading.RateLimiting`
10.0.10 · `Microsoft.Extensions.Http.Resilience` and
`Microsoft.Extensions.TimeProvider.Testing` 10.8.0 ·
`Microsoft.NET.Test.Sdk` 18.8.1 · `Microsoft.SourceLink.*` 10.0.301 ·
`Nerdbank.GitVersioning` 3.10.91 · `Serilog` 4.4.0 ·
`System.CommandLine` 2.0.10 · `Avalonia`/`Avalonia.Desktop` 12.1.1 ·
`MQTTnet`/`MQTTnet.Server` 5.2.0.1603 (modern band only) ·
`Roslynator.*` 4.16.0 · `TUnit` 1.64.6.

The two `System.Collections.Immutable` `VersionOverride`s in the schema
test projects move to 10.0.10 to stay ahead of what `JsonSchema.Net`
requires.

## Removed: the DotNext pin

`DotNext` 5.26.3 was pinned centrally but no project references it. The
`docs/migrate/2.0.x/packages.md` row claiming `src/Opc.Ua.Lds.Server`
consumes it was also wrong — that project references only
`Makaretu.Dns.Multicast` and
`Microsoft.Extensions.Hosting.Abstractions`. Both are corrected.

## BouncyCastle held at 2.6.2 — CI caught a real regression

The first push of this branch took `BouncyCastle.Cryptography` to 2.7.0.
CI failed `Opc.Ua.Core.Tests` on **net48 only**
(`VerifyPEMSupportDirectoryStoreAsync` expecting 3 certificates and
getting 0), and the cause turned out to be a genuine behavioural break
rather than a flaky test.

BouncyCastle is referenced **only on net472/net48**, where it backs
`PEMReader`/`PEMWriter` in `Opc.Ua.Security.Certificates`. 2.7.0
tightened `Asn1.X509.TbsCertificateStructure` to throw:

```
certificate issuer is an empty distinguished name
```

`OpenSsl.PemReader.ReadObject()` therefore aborts on the *first* entry
of a chain whose issuer DN is empty. `ImportPublicKeysFromPEM` has no
per-object recovery, so the entire file is discarded and the store
enumerates nothing.

This is a **runtime** behaviour change, not a test artefact: .NET's own
PEM support on net8.0+ still accepts such a certificate, so taking 2.7.0
would let the same trusted-peer store load fine on net8.0+ and come back
**empty** on .NET Framework. Whether the stack *should* reject these
certificates on every TFM is a fair question, but it is a
security-behaviour decision that does not belong in a dependency refresh
— so the pin is documented in place with that context for a follow-up.

Confirmed by bisecting the package version against the net48 tests
(2.6.2 passes, 2.7.0 fails) and by reading the actual PEM asset with
both versions to capture the exact throw site.

## Deliberately left alone

Every existing pin with a documented reason is preserved, and each was
re-checked rather than assumed:

- **Roslyn compat band** (`roslyn.props`) — `Microsoft.CodeAnalysis.*`
stays on the pinned band and
`System.Collections.Immutable`/`System.Reflection.Metadata` stay on
`$(RoslynRuntimeVersion)`. Raising these is what re-breaks the analyzer
`ImmutableArray<T>` identity the file warns about, and it would raise
the minimum compiler that will load the shipped analyzers.
`Microsoft.CodeAnalysis.Analyzers` stays at 4.14.0 for the same reason.
- **`Microsoft.AspNetCore.*` 2.3.11** — last band with a
`netstandard2.0` asset; already latest.
- **`PacketDotNet` / `SharpPcap`** — SDL native-code pins; already
latest stable.
- **`System.Net.Http` 4.3.4 / `System.Text.RegularExpressions` 4.3.1 /
`System.Private.Uri` 4.3.2** — transitive CVE remediation pins; already
latest.
- **`MQTTnet` 4.3.7.1207** for `net472`/`net48`/`netstandard2.1` —
MQTTnet 5.x dropped those TFMs.
- **`BouncyCastle.Cryptography` 2.6.2** — see the section above.
- **`Moq`** — pinned.
- **Prerelease-only upgrades skipped** — `BenchmarkDotNet`
0.16.0-preview, `NUnit` 5.0.0-beta, `NUnit.Console` 4.0.0-beta,
`EmbedIO` 4.0.0-rc1, `Newtonsoft.Json` 13.0.5-beta, `PacketDotNet`
1.4.9-pre, and others.

## Overlap with #4195 — please note

**OpenUSD and ModelContextProtocol are intentionally not touched here.**
#4195 already moves OpenUSD to `0.6.0-alpha` together with the restored
render smoke job and the `OpenUsd.Rendering.Silk.Vulkan` package that
the bump actually depends on, and separately moves MCP to `2.1.0`.
Rev'ing either in this PR would duplicate that work, and would risk
silently downgrading MCP if the two landed in the wrong order. They are
left to #4195.

There is still a small expected overlap in `Directory.Packages.props`:
#4195 also bumps `Microsoft.Extensions.Configuration.Abstractions`,
`Microsoft.Extensions.Hosting.Abstractions` and `System.Text.Json` to
10.0.10 — the same values as here, so whichever merges second resolves
to an identical line.

## The one code change

Roslynator 4.16 adds **RCS1258**, which flags `TypeAttributes.Public |
TypeAttributes.Class` in `ComplexTypeBuilder`. It is correct:
`TypeAttributes.Class` is `0`, so the flag is a no-op, and the
`DefineEnum` call directly above already passes `TypeAttributes.Public`
alone. Dropping it is a verified semantic no-op (`Public | Class ==
Public`).

## Related Issues

_No tracking issue — routine dependency maintenance, in the same spirit
as the weekly Dependabot NuGet updates configured in
`.github/dependabot.yml`._

## 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.<br/>_No new tests: this
changes dependency versions, not behaviour. The existing suite is the
proof, and the one code change is a verified no-op._
- [x] I have added all necessary
documentation.<br/>_`docs/migrate/2.0.x/packages.md` — corrected version
rows, removed the wrong `DotNext` row, added a section explaining the
per-TFM ASP.NET Core scheme._
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.<br/>_Full `UA.slnx` build across all six TFMs
(`net472`, `net48`, `netstandard2.1`, `net8.0`, `net9.0`, `net10.0`):
**0 warnings, 0 errors**. `dotnet restore`: 0 warnings. `dotnet pack`: 0
warnings, 0 errors, package validation passes._
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.<br/>_See
the verification note below._
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

### Update after the first CI run

The initial push failed one CI job — `Opc.Ua.Core.Tests` on net48, 2 PEM
tests. That was **not** flakiness: it was a real break from BouncyCastle
2.7.0, root-caused and fixed by holding that package at 2.6.2 (see the
section above). `Opc.Ua.Core.Tests` on net48 now runs **4161 passed / 0
failed** locally, and the full six-TFM build is still 0 warnings / 0
errors.

### Review feedback

Both review threads are addressed in `78bdd0ea3`:

- The **package table's third column was stale** in more rows than the
ones flagged. Every entry is now checked against the actual
`PackageReference` items — `System.Threading.Channels` belongs to
`Opc.Ua.Core`/`Core.Diagnostics`/`PubSub.Diagnostics` (not the LDS
server), the `Microsoft.CodeAnalysis.*` rows come from the
generator/analyzer variant targets under `tools/`, and
`Microsoft.CodeAnalysis.Analyzers` plus `System.Reflection.Metadata`
have no direct reference at all and are labelled as central pins. The
column is renamed **"Referenced by"**, which is what it actually records
and what makes it verifiable.
- The **BouncyCastle pin now references
[#4224](https://github.com/OPCFoundation/UA-.NETStandard/issues/4224)**,
which carries the decision on whether the stack should reject
empty-issuer certificates on every TFM, whether
`ImportPublicKeysFromPEM` should recover per entry, and whether the
expired `Test_chain.pem` asset should be regenerated.

## Verification note on the test run

The full net10.0 run covered ~48,000 tests. 15 failed, and each was
individually shown **not** to be caused by this change:

- **4 in `Opc.Ua.Core.Diagnostics.Tests`** (3×
`ReadChunkDecryptsStackEncryptedSymmetricChunk`,
`PrivateRequestSummaryFormatsKnownRequestKinds`) — reproduced
**identically at `HEAD` with the changes stashed**. Pre-existing.
- **8 across `Client`, `Di`, `WotCon.Samples`, `Server`** — all pass
when their project is run in isolation; they only fail under the
parallelism of a full-solution run. The machine also had ~45 orphaned
test processes holding sockets, which were cleared.
- **3 in `Gds`/`Stress`** — the log shows socket port exhaustion (`Only
one usage of each socket address...`). The `CertRotationLiveTests` pair
passes when run standalone, and that project's net10.0 package closure
contains **none** of the bumped packages (`BouncyCastle` is
`net472`/`net48`-only, so it cannot reach a net10.0 test at all).

The AOT/TUnit suite (`Opc.Ua.Aot.Tests`) was A/B tested: TUnit **1.58**
produced the same intermittent 0–2 failures as 1.64, confirming
pre-existing flakiness rather than a regression. On **1.64.6** it ran
**129/129 twice**.

`net48` was built and exercised via `CustomTestTarget=net48`; the
`MSB3277`/`NU1702` warnings it surfaces in `Opc.Ua.Stress.Tests` are
structural to that mode (the project builds `net8.0` while consuming
`net48` outputs) and pre-date this change — the `net462` asset's
assembly version simply tracks the patch number, so `10.0.0.9` became
`10.0.0.10`.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2be367ab-e62d-4141-a4df-e5b2ca3cd126
 2026-08-10 18:18:27 +00:00
CopilotmarcschierMarc Schier
 		404517c6e8 [MigrationAnalyzer] Generate public collection shims to unblock incremental 1.5.378 → 2.0 migration (#4214) 
# Description

MigrationAnalyzer-generated `<Type>Collection` shims were emitted as
`internal`, which breaks consumers exposing these types in public APIs
during migration. This change makes generated shims `public` while
preserving `[Obsolete]` + `UA0002` guidance.

- **Generator output access level**
  - `MigrationGenerator` now emits:
    - `public sealed [Obsolete] class <Name>Collection : List<TElement>`
- No behavior changes to element-type resolution, deduplication, or
diagnostics (`MIG01`).

- **Targeted generator test updates**
- Updated assertions in `MigrationGeneratorTests` to validate `public
sealed class ...` output for:
    - well-known overrides,
    - semantic lookup for model types,
    - semantic lookup for built-in and primitive element types.

- **Migration docs alignment**
- Updated migration analyzer docs to reflect public shim generation and
removed outdated internal-only guidance.

```csharp
[global::System.Obsolete("... (UA0002)")]
public sealed class Int32Collection : global::System.Collections.Generic.List<int>
{
    public static implicit operator global::Opc.Ua.ArrayOf<int>(Int32Collection? value)
        => value is null ? default : value.ToArrayOf();
}
```

## Related Issues

- OPCFoundation/UA-.NETStandard#4205

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

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

- Fixes #4205

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
 2026-08-10 16:28:56 +00:00
Marc SchierandCopilot e05a820c91 Removal of the exposed diagnostics and node locks (#4188) 
## What this is

Design analysis for `Opc.Ua.Server`, the two rounds of design
exploration behind it, and the first two implementation chunks it
recommends.

Draft: the analyses are ready to read, but the *conclusions* deserve
discussion before the larger phases proceed. Later chunks stack onto
this PR as they land.

| File | Kind | Lines |
|---|---|---:|
| `plans/NodeManagerAnalysis.md` | Evidence — the node-manager plugin
surface | 488 |
| `plans/ServerRuntimeAnalysis.md` | Evidence — the server runtime
cluster | 576 |
| `plans/ServerDesignExploration.md` | Design round 2 — delivery
strategy | 1,394 |
| `plans/ServerDesignExplorationAlt.md` | Design round 1 — four
interface designs | 2,119 |
| `src/Opc.Ua.Types/State/NodeState.cs` | **Code** — node self-guards
attributes | |
| `src/Opc.Ua.Types/State/BaseVariableState.cs` | **Code** — value
accessors take the lock | |
| `src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs` | **Code** —
6 external locks removed | |
| `tests/…/NodeStateAttributeConcurrencyTests.cs` | **Code** —
concurrency safety net | 449 |

`docs/` is untouched — it holds shipped feature documentation, and none
of this is shipped API.

## The analyses

Counted mechanically over `src`, `samples` and `tests`; **every figure
is reproducible** from commands printed in each document.

**Node manager surface.** `AsyncCustomNodeManager` declares **93
`virtual`/`abstract` members** (80 distinct names). Across the
repository: **112 override sites, 25 distinct names ever overridden, 55
of 80 never**. Six carry nearly all real use. For scale,
`ReferenceNodeManager` is 6,373 lines and overrides **8**.

**Server runtime.** Seven findings, **two of which recommend no
change**. Findings 1–5 are filed as #4183–#4187. The headline is an
`object`-typed sync root published on three public interfaces and taken
in **88 `lock` statements across 7 files**, including a shipped sample.

**What is deliberately not flagged.** `IStandardServer` — 9 members over
5,575 lines, 16 of 37 virtuals used — is identified as the reference
shape to preserve. `IApplicationInstance` and the already-`[Obsolete]`
`Stop()` are examined and left alone.

## The design exploration

**Round 1** explored four alternative interfaces in parallel under the
current back-compat constraint. Its value is the convergence: four
designers who never saw each other's work independently arrived at a 6–9
member `IServerContext` replacing the 57-member locator, removal of the
`object` node-manager handle, elimination of the `Processed` protocol,
and `IHistorianProvider` as the pattern to copy.

**Round 2** re-runs it with back-compat lifted and asks how to deliver:
full rewrite versus side-by-side fork. **Both option designers
independently declined to recommend their own assignment.** Three
structural facts were verified:

1. **The fork line already sits at `IAsyncNodeManager`** — old and new
node managers can coexist with zero runtime changes, so a fork
duplicates ~10% of the assembly, not all of it.
2. **`[OpcUaShim]` cannot shim an inheritance surface** — honest
auto-fix ceiling for the authoring surface is **~10%**, against ~70% for
the shipped analyzers.
3. **A second lock leak exists** — `NodeState` takes `lock (this)` under
a `CA2002` suppression whose own comment documents the contract.

Recommendation: **neither pure option**, but a staged path reaching the
rewrite end state incrementally, gated on migrating the five
framework-internal node managers that today make it impossible for the
old base to ever freeze.

## Implementation: NodeState now guards its own attributes

This removes the prose-only contract in which `NodeState` took
`lock(this)` and every external caller was required to take
`lock(source)` on the node instance. The code already carried a TODO
describing exactly this fix and noting it required updating every
external site.

- The node self-guards rather than relying on callers.
`BaseVariableState` gains a **fully private** `System.Threading.Lock`
covering its value fields; `NodeState`'s other collections keep their
existing private locks.
- All **6** `lock(this)` sites and both `CA2002`/`RCS1059` suppressions
are gone.
- `Value`, `Timestamp` and `StatusCode` on `BaseVariableState` take the
lock too — without that, a direct property write raced an attribute read
once the external lock was gone.
- **6 of 9** external sites removed. Five were already redundant
(`GetReferences`, `GetChildren`, `ReferenceExists` and
`SetAreEventsMonitored` guard themselves; `FindMethod` reaches its state
via `GetChildren`); the sixth was an attribute read now covered by the
node's own lock.
- Attribute read/write deliberately take **no** node-wide lock: both
dispatch to caller-supplied handlers (`OnReadValue` and siblings) that
may take locks of their own, and holding a lock on the node across one
gives the node no fixed place in the lock order.
- **3 deliberately retained and annotated in code**: two `CreateBrowser`
calls, which are not internally synchronized (tracked in #4216), and one
in `CustomNodeManager`, whose class is slated for deletion so
refactoring its lock would be wasted work.

### No lock is exposed anywhere, to anyone

An earlier revision put the attribute lock on `NodeState` as `private
protected`, which still published a lock-sharing contract on the
inheritance surface. Review pushed back and it is gone:
`BaseVariableState` was the only user, so the lock moved there as a
plain `private readonly Lock`. There is no member for a derived type to
call, in this assembly or any other.

`BaseVariableValue` was the last public leak — it handed out the lock
guarding its value through a `public Lock` property, so any caller could
hold it across a call back into the stack. That property is removed:

- Derived value classes (what the source generator emits for every
structure variable) synchronize through a `protected EnterLock()` /
`ExitLock()` pair. Callback helpers were the first choice, but the
generated bodies pass `ref Variant` / `ref StatusCode` / `ref
DateTimeUtc`, and `ref` arguments cannot be captured by a lambda. The
lock *object* stays private either way.
- `NodeStateTemplates` emits the new form.
- `ServerInternalData`, the one cross-assembly caller, was reading the
lock back off a value it had itself constructed with that lock. It now
takes `m_diagnosticsLock` directly.

`NodeBrowser.DataLock` remains `protected` and is tracked in #4215: a
subclass extending `Next()` legitimately needs its enumeration state
atomic with the base browser's, so the fix needs a base-owned
acquisition rather than a deletion.

### Migration coverage for the removed lock members

None of the removed lock members get an `[Obsolete]` shim, and the
migration guide now says why: a shim would have to be `object`-typed to
keep old call sites compiling, and on .NET 9+ `lock (o)` on a
statically-`object` reference is `Monitor.Enter`, which does **not**
interlock with `Lock.Enter()`. It would compile, run, and silently
synchronize nothing — strictly worse than a compile error. Re-adding a
member to a downstream-implemented interface is also a break in its own
right on `net48` / `netstandard2.0`, where there are no default
interface methods.

Instead, every removed member has a named replacement on the migration
path:

| Rule | Covers | Replacement it names |
|---|---|---|
| `UA0024` | `IServerInternal` / `ISession` / `ISubscription`
`DiagnosticsLock`, `DiagnosticsWriteLock` |
`UpdateServerDiagnostics(...)` / `UpdateDiagnostics(...)`,
`ReadServerDiagnostics` / `ReadDiagnostics` |
| `UA0025` | `ILocalNode.DataLock`, `Node.DataLock` | a lock the caller
owns; the node guards its own state |
| `UA0026` | `BaseVariableValue.Lock` | a lock passed to the
`BaseVariableValue` constructor |

Each rule fires on sources still written against 1.5.378, where the
compiler only says a member is missing. `docs/MigrationGuide.md` gains a
section per removal with before/after.

### The tests earned their keep

The four concurrency tests were landed **first**, against the unmodified
code, because removing a synchronization contract cannot be verified by
the compiler and the existing `NodeStateHandlerConcurrencyTests` covers
a different concern (racing a handler assignment against a write, not
reader/writer exclusion). They now run with **no external locking**,
which is what demonstrates the node guards itself.

They caught two real problems, and one of them was mine:

- The `Value`/`StatusCode`/`Timestamp` setters bypassed the new lock — a
genuine race.
- One test asserted an invariant the design does not offer. Two property
setters are two lock acquisitions, so that pair was **never** atomic; it
only passed originally because the test held a lock across both. It now
uses a single attribute write carrying a whole `DataValue`, which
genuinely is atomic and is the path the server uses.

A third issue was found and fixed before it shipped: the reader loops
gated on `!writer.IsCompleted`, so a reader scheduled after the writer
finished performed zero reads — failing two runs in three on `net48`.
Converted to `do…while`; now five consecutive green runs.

**Verified:** `Stack.State` 116 passed on `net10.0` *and* `net48`; the
concurrency category green 5/5 consecutive on `net48`;
`Opc.Ua.Types.Tests` 8,456 passed; the server node-manager suite 1,205
passed, including `NodeManagerLifecycleTests` and
`AsyncCustomNodeManagerTests` which exercise the changed paths.

## Two corrections to my own published figures

Both were found while starting implementation, and both are recorded on
#4183.

**`ForceDiagnosticsScan()` was overstated.** The analysis claimed the
`DiagnosticsWriteLock` getter *"triggers a full address-space
diagnostics rescan"*. It is in fact a single flag assignment
(`DiagnosticsNodeManager.cs:700-703`) consumed by the read paths and the
periodic timer. The defect stands — a side-effecting property getter
with a racy write on a synchronisation path — but its cost was
misstated.

**The contract was 15 sites, not 19.** A naive search matched ten
comment and `#pragma` lines that *mention* `lock(source)` while
explaining it. The documents now carry a counting command that excludes
comment matches.

A third finding expanded scope the other way: the `object`-typed lock
leak is **repo-wide**, with ten such members across `Opc.Ua.Core`,
`Opc.Ua.Types` and `Opc.Ua.Server` — including `Node.DataLock => this`
and `ApplicationConfiguration.PropertiesLock => m_properties`, the same
defect class as `Subscription.DiagnosticsLock`.

## Caveats stated in the documents

- **Nothing in the designs is implemented or benchmarked.** Every
performance claim across all seven designs is a design intention, not a
measurement.
- The larger phases are a **major undertaking against a live plugin
API**; the documents say so rather than letting the neatness of the
proposals imply otherwise.
- The recommended sequencing puts low-risk, self-contained steps first
and notes they are **worth doing regardless** of which strategy is
eventually chosen.
- Round 2 notes the two option designers' cost estimates are **not
independent of their assignments**.

## Review guidance

The two analyses make factual claims checkable by running the included
commands. The design explorations are exploratory and need not be agreed
with to merge. The code change is small, test-backed, and independent of
which delivery strategy is eventually chosen.

Reasonable outcomes include merging as-is, merging only the analyses and
the lock fix, or disagreeing with the recommendation while keeping the
evidence.

---

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041
Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63
 2026-08-10 06:47:50 +02:00
Marc SchierandCopilot b17007bd55 Stop the encoder tests flooding the VSTest channel with per-test output (#4219) 
# Description

The `Core.Encoders` test job of run
[`31249610490`](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/31249610490)
printed nothing for 57 minutes and was killed by the 60-minute job
timeout. Issue #4213 recorded that as "hung at or before the first test"
— that reading was wrong, and the evidence to disprove it was already in
the job's own artifact.

The blame collector *had* written a hang dump and it *was* uploaded,
inside the ordinary `dotnet-results-Core.Encoders-*` artifact (11 MB of
it was the dump), where nobody looked for it. Opening it names the cause
precisely.

## What the dump shows

522 threads (408 background) in a process that finishes in 25 seconds
when healthy. Every test had already run; NUnit was in `OneTimeTearDown`
waiting on its event pump:

- the **event pump thread** is blocked in
`NUnitEventListener.TestFinished` →
`TestRunEventsHandler.HandleLogMessage` →
`LengthPrefixCommunicationChannel.Send` → `Socket.Send`, on a socket
that never drained. That send holds the channel's lock;
- `TestRunCache.SendResults` is blocked on that channel lock, and it
holds the cache lock;
- `TestRunCache`'s **one-second timer** then fired repeatedly into that
held cache lock, and the thread pool injected a fresh worker for each —
**399 threads** parked in `Monitor.Enter_Slowpath` behind a single lock;
- the NUnit run thread sits in `EventPump.Stop()` → `Thread.Join()`, so
the run can never complete.

## What put the socket under that much pressure

Volume of test output. Every encoder case dumps its input, its encoded
payload and its decoded result, the NUnit adapter forwards each captured
line to the runner as an individual message over that socket, and the
suite has 2929 cases. This is not marginal:

| | Before | After |
| --- | --- | --- |
| `Core.Encoders` TRX, green local run | **496 MB** | **4 MB** |
| `Core.Encoders` published CI artifact | **156 MB** | ~3 MB |
| Next-largest test job's artifact, for scale | 15 MB | 15 MB |
| `Client.ComplexTypes` TRX | 74 MB | 40 MB |

So the suite was shipping half a gigabyte of text through a socket, one
message per line, on every run and on every OS — and paying for it in
artifact storage even when nothing went wrong.

## The change

`EncoderCommon` gains a buffered `TestOutput` writer. The dumps are
collected per test and handed to `TestContext.Out` in `TearDown` **only
when the test did not pass** — which is the only time anyone reads them.
Nothing is lost: a failing test still reports its full
input/encoded/decoded dump under *Standard Output Messages*, verified by
injecting a deliberate failure. Explicit opt-ins
(`PrettifyAndValidateJson(..., outputFormatted: true)`) and the static
helpers keep writing immediately.

The same base class is shared by the `Client.ComplexTypes` fixtures, so
those are moved over too rather than left with half-buffered,
half-immediate output.

Two supporting changes:

- **`.github/workflows/buildandtest.yml`** — `timeout-minutes: 30` on
the `Test` step. The blame collector did its job here (dump at +50 min,
once the run went quiet), but the run still had not unwound 7 minutes
later when the *job* timeout cancelled everything, so the step needs a
bound below the job's. `Upload test results` already runs `if:
always()`, so the dump is still published.
- **`docs/DeveloperGuide.md`** — a "Test output" convention under
*Running tests*, so the next data-driven fixture does not re-introduce
this.

## Notes for reviewers

- `[Parallelizable]` on these fixtures is `ParallelScope.Self`, i.e.
fixture-level; tests inside a fixture stay sequential, so the
per-fixture buffer is as safe as the existing per-fixture
`RandomSource`/`DataGenerator` state that is already reset in `SetUp`.
- I separately confirmed that `--blame-hang-timeout` *does* fire on a
hang before the first test (minimal NUnit repro, dump at exactly the
configured timeout), which is what rules out the original "hung during
startup/discovery" theory independently of the dump.
- Local verification was scoped to the two affected suites rather than
the whole solution: `Opc.Ua.Core.Encoders.Tests` 2929/2929 on
**net10.0** *and* **net48**, `Opc.Ua.Client.ComplexTypes.Tests`
2773/2773 on net10.0, all building with 0 warnings. The full `UA.slnx`
run is left to CI.

## Related Issues

- Fixes #4213

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: da92151a-3da2-4caf-a39c-5556e56f0506
 2026-08-09 21:49:05 +02:00
Marc SchierandCopilot 0315239380 Register the PKCS#11 package in the expected package set (#4218) 
## Description

`validate-source-generator-packages.ps1` fails the build when the packed
output does not match `.azurepipelines/expected-packages.txt` exactly.
The point, in its own words, is that adding, removing or renaming a
shipped package has to be a conscious, reviewed act — and that a private
build-time project accidentally becoming packable is caught rather than
shipped.

#4192 added `OPCFoundation.NetStandard.Opc.Ua.Security.Pkcs11` without
registering it, which is exactly what that check exists to catch. This
registers it.

## This does not turn the job green on its own

The list is stale for **twenty** other packages, all from earlier
changes:

`Mcp.Core`, `Mcp.Diagnostics`, `Mcp.PubSub`, `Mcp.PubSub.Diagnostics`,
`OpenUsd`, `OpenUsd.Client`, `OpenUsd.Connector`,
`OpenUsd.Connector.Viewer`, `OpenUsd.Server`, `OpenUsdScene`,
`OpenUsdScene.Server`, `Positioning`, `Positioning.Client`,
`Positioning.Server`, `Robotics`, `Robotics.Client`, `Robotics.Server`,
`WotCon.Bindings`, `WotCon.Bindings.Mqtt`

I deliberately did not add those. Asserting that someone else's package
is intentionally shipped is the one judgement this check is designed to
force a human to make, so batching them in would defeat it. `Build,
sign, pack, publish` has been failing on `master` for this reason since
before #4192 (visible on `5a6bbcce1`); this removes one of the
twenty-one causes, the one that is mine.

## Related Issues

- Follow-up to #4192

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

### Tests

None needed — this is a one-line data file consumed by
`validate-source-generator-packages.ps1`, and that script *is* the test.
The package it registers is exercised by the 96 PKCS#11 tests added in
#4192.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0502132e-320a-48dc-84e2-bcd8d854aa70
 2026-08-09 21:42:01 +02:00
Marc SchierandCopilot f23b30e81a Own the certificates the TLS validation callbacks inspect (#4196) 
Fixes the `test-ubuntu-latest-Sessions` failure that is currently red on
**master** ([run
31246003974](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/31246003974),
commit `5a6bbcce1`) and therefore on every pull request cut from it.

## The failure is not a failing test

`Opc.Ua.Sessions.Tests` passes — `Failed: 0, Passed: 777, Skipped: 379`.
The `dotnet test` step then exits **1** anyway, because the repository's
certificate-leak check runs after the suite and finds one outstanding
`Certificate` instance. The `BadUserAccessDenied` / *"Too many failed
authentication attempts"* noise in the log is a red herring: those
messages appear in the **passing macOS** run of the same suite too, and
are the expected product of tests that deliberately fail authentication.

## Cause

The TLS server-certificate validation callbacks built their OPC UA
`CertificateCollection` by wrapping the `X509ChainElement.Certificate`
instances the platform handed them. Those handles are **borrowed** — the
`X509Chain` owns them and frees them when it is disposed — so wrapping
them put two owners on one handle, and one certificate instance survived
to process exit.

It is timing-sensitive rather than deterministic, which is why macOS
passes and Linux does not, and why it presents as an intermittent leak
rather than a hard failure.

## Change

Each callback now copies the raw DER and builds the collection from
that, so the collection owns what it disposes and the chain keeps what
it owns:

- `src/Opc.Ua.Bindings.Https/Https/WebSocketByteTransport.cs`
- `src/Opc.Ua.Bindings.Https/Https/WssJsonTransportChannel.cs`
- `src/Opc.Ua.Client/WebApi/WebApiTransportChannel.cs`
- `src/Opc.Ua.Client/WebApi/WebApiWssTransportChannel.cs`
- `src/Opc.Ua.Core/Stack/Https/HttpsTransportChannel.cs`

**The validation decision is unchanged.** The same certificates are
inspected, in the same order, by the same logic — only their lifetime
differs. This is security-relevant code, so it is worth stating plainly
that nothing about what is accepted or rejected has moved.

`LeakDetectionHelpers` also gets its ceiling raised from 10s to 60s,
**keeping its quiescence semantics**: progress still resets the clock,
and a count that has stopped moving is still reported without waiting
for the cap. The settled window goes from 0.5s to 5s — long enough not
to race a loaded Linux agent's transport teardown, still prompt enough
to report a real leak quickly. An earlier attempt at this set the
settled window equal to the cap, which would have turned the check into
a flat 30-second sleep and destroyed its ability to tell a genuine leak
from a slow drain; that was rejected.

## Validation

`dotnet build UA.slnx` — 0 errors, 0 warnings.

| Suite | TFM | Passed | Failed | Exit |
|---|---|---:|---:|---:|
| `Opc.Ua.Sessions.Tests` | net10.0 | 776 | 0 | 0 |
| `Opc.Ua.Sessions.Tests` | net48 | 599 | 0 | 0 |
| `Opc.Ua.Bindings.Https.WebApi.Tests` | net10.0 | 337 | 0 | 0 |
| `Opc.Ua.Client.Tests` | net10.0 | 2123 | 0 | 0 |
| `Opc.Ua.Core.Tests` | net10.0 | 4153 | 0 | 0 |

7,988 passed, 0 failed. **Exit code 0** everywhere — which is the signal
that matters here, since the original bug was a suite that reported
all-pass and still exited 1. No `Certificate leak detected` message in
any run.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
 2026-08-09 12:55:03 +02:00
Marc Schier 0612a43fe4 Pluggable cryptography and hardware-held private keys (#4192) 
Makes the stack's cryptographic operations replaceable - by another library, an offboard service, or hardware (TPM 2.0 / HSM / PKCS#11 token / cloud KMS) - with no performance cost in the default configuration, and extends this to certificates so a private key need never exist in process memory.

Also fixes three pre-existing defects found while validating: STACKGEN001 erroring for every compilation the stack generator was merely loaded into, certificates failing to load from paths past MAX_PATH on Windows, and the MonitoredNode2 tests starving the thread pool.

Fixes #4190
 2026-08-09 12:40:26 +02:00
Copilotandmarcschier d8aca69444 Remove unused MinMetadataSamplingInterval configuration option (#4194) 
## Description

Server-side `MonitoredItem` sampling interval revision is based on the
per-node `BaseVariableState.MinimumSamplingInterval` attribute, not on
any global server setting —
`ServerConfiguration.MinMetadataSamplingInterval` was dead configuration
that was never read by any code path. Per maintainer feedback, this PR
removes it instead of wiring it into revision logic that was never meant
to consume it.

Separately, the concern that `Subscription.MaxNotificationsPerPublish`
isn't revised by the server was verified against existing coverage:
`CreateSubscriptionWithNotificationLimitsUsesEffectiveLimitAsync` /
`ModifySubscriptionWithNotificationLimitsUsesEffectiveLimitAsync` in
`SubscriptionTests.cs` already assert the server clamps a
client-requested value down to the configured server limit (e.g.
requested `25` vs. server limit `10` → revised `10`).

### Removed `MinMetadataSamplingInterval`
- Dropped the `ServerConfiguration.MinMetadataSamplingInterval` property
(`ApplicationConfiguration.cs`) and its XSD element
- Dropped `SetMinMetadataSamplingInterval` from
`IApplicationConfigurationBuilder` / `ApplicationConfigurationBuilder`
- Removed the element from sample and test server configuration XML
files (`Ctt.ReferenceServer.Config.xml`,
`Quickstarts.ReferenceServer.Config.xml`, GDS test configs,
`test-server-full.xml`)
- Updated `ApplicationConfigurationBuilderTests` and
`ConfigurationRoundTripTests` to drop assertions on the removed property

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

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

- Fixes #4191

---------

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-08 17:11:42 +02:00
Tim JöhnkandClaude Opus 5 5a6bbcce1c Add session and subscription ids to client and server request logging (#4189) 
## Proposed changes

Request-path log messages in the client and server carry no session
context, which makes
them unusable for diagnosing an application that holds several sessions
at once — a
gateway, an aggregating client, or any host with more than one
connection. Lines like

    PUBLISH #4711 SENT
    NOTIFICATION RECEIVED: SubId=12, SeqNo=980
    Server Call=Publish, Id=8123

interleave across sessions in one log file with nothing to separate
them. The same applies
on the server side to messages that name a monitored item but not its
subscription.

This adds `SessionId` — and `SubscriptionId` where the call site has one
— to the existing
source-generated log messages in the client and server request paths.

Scope is deliberately narrow. The change touches **message templates and
parameter lists
only**:

- no new log statements, no changes to log levels, event ids or event
names
- no public API change: every changed method lives in an `internal
static partial class
  …Log` type
- no behavioural change beyond what is logged

11 source files and 1 test, +531/−259. 81 messages gain an id.

| Area | Files |
| --- | --- |
| Client | `Session/Session.cs`,
`Session/Subscription/ClassicSubscriptionEngine.cs`,
`Subscription/Classic/Subscription.cs`,
`Subscription/Classic/MonitoredItem.cs` |
| Server | `Server/StandardServer.cs`,
`Subscription/SubscriptionManager.cs`, `Subscription/Subscription.cs`,
`Subscription/SessionPublishQueue.cs`,
`Subscription/MonitoredItem/MonitoredItem.cs`,
`Subscription/MonitoredItem/QueueHandler/DataChangeQueueHandler.cs`,
`MonitoredItemQueueLog.cs` |

### Points worth a reviewer's attention

**Two legacy-contract events gain a field.**
`ClientEventSubscriptionState`
(`LegacySubscriptionStateId` / `"SubscriptionState"`) and
`ClientEventNotificationReceived`
(`LegacyNotificationReceivedId` / `"NotificationReceived"`) now carry
`SessionId`. Their
event ids, event names and levels are unchanged, and every property
`OpcUaClientCompatibilityLoggingTests` pins is still present — the test
is extended to pin
the new one as well. If you would rather the legacy-shaped events stay
byte-identical, say
so and I will drop the id from those two; the sibling non-legacy
messages at the same call
sites already carry it.

**`DEQUEUE VALUE` gains only `MonitoredItemId`, not a subscription id.**
Its message is
shared between `MonitoredItem` and `DataChangeQueueHandler`, and the
latter holds a
monitored item id but has no subscription or session in scope. The
message previously
identified nothing at all; adding the id it can actually supply seemed
better than
inventing a subscription id at one of the two call sites.

**`QUEUE OVERFLOW` in `SentMessageQueue` is left alone.** That class is
constructed with a
`Func<uint> subscriptionIdProvider` and no session, so there is no
session id to add. The
message already carries `SubId={SubscriptionId}`.

**Server trace states made consistent.** `Subscription.TraceState` has
four cases; `Deleted`
and `Config` already named their session, `Items` and
`Publish`/`Monitor` did not. All four
do now.

**One local added** in `Subscription.RestoreTriggeringAsync`: `ISession
session = Session;`
before the `foreach`. `Session?.SessionId` inside that loop does not
compile — `?.` is a
null test, so it resets the nullable flow state for `Session`, and the
`foreach` back-edge
carries that state to the top of the loop body, which makes the
pre-existing
`Session.SetTriggeringAsync(...)` a CS8602 error. Capturing the
reference once avoids
disturbing existing code and is more useful than `?.` anyway: it reports
the session the
call was actually made on rather than a blank.

**Publish responses are attributed to the session that sent the
request.** In
`ClassicSubscriptionEngine.OnPublishComplete` the added arguments read
the `sessionId`
parameter rather than `m_context.SessionId`, so a response arriving
after a reconnect is
still attributed to the session that produced it.

### Null-safety of the added arguments

Reviewed at every call site:

- Client `Subscription.Session` is `ISession?` and is cleared when a
subscription is removed
from its session, so those arguments are `Session?.SessionId` or read
from the captured
local. `[MemberNotNull]` on `VerifySession()` /
`VerifySessionAndSubscriptionState()`
satisfies the compiler but only describes the moment the guard returns,
not the state
after the following `await` — and several of these arguments sit in
`catch` blocks, where
  an NRE would have replaced a `ServiceResultException` and escaped.
- Client `Session` derives `SessionId` from `SessionClient`, which is
non-nullable.
- Server `OperationContext.SessionId` and `Subscription.SessionId`
already return `default`
  when there is no session, so those arguments cannot throw.
- Consequence worth knowing: when the server reaps expired
subscriptions,
`Subscription.DeleteAsync` synthesises an `OperationContext` with no
session, so
  `SessionId` logs empty on that path. That is expected, not a defect.

## Related Issues

None — no existing issue covers this. Happy to open one for tracking if
you prefer.

## Types of changes

- [x] Enhancement (non-breaking change which adds functionality)
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist

- [x] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [x] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines.
- [ ] I fixed all introduced issues with CodeQL and LGTM.
- [x] I have added necessary documentation (if appropriate).

`Opc.Ua.Client` and `Opc.Ua.Server` build clean for every target
framework
(net472, net48, netstandard2.1, net8.0, net9.0, net10.0) with
`--no-incremental`:
0 errors, 0 warnings under the repository's `TreatWarningsAsErrors`.

Local results, Release/net8.0:

| Suite | Result |
| --- | --- |
| `Opc.Ua.Server.Tests` | 0 failed, 4024 passed, 5 skipped |
| `Opc.Ua.Subscriptions.Classic.Tests` | 0 failed, 32 passed, 0 skipped
|

`Opc.Ua.Client.Tests` and `Opc.Ua.Sessions.Tests` are still running
here; I will post their
results as a comment rather than hold the PR for them.

The test output also confirms the placeholders bind at runtime, which
compilation alone does
not show — for example, from the transfer tests:

[Opc.Ua.Client.ClassicSubscriptionEngine] Publish #31,
Reconnecting=False,
      Error: ... (BadNoSubscription), SessionId=ns=3;i=1187191890

No tests are added beyond the compatibility-test extension: the change
alters message
templates and arguments, which the suites otherwise do not assert on.
Happy to add coverage
if you would like a specific message pinned.

The CI and CodeQL boxes are left blank deliberately — per CONTRIBUTING
the pipelines do not
start for outside contributors until a maintainer comments `/azp run`,
so there is nothing
for me to act on yet.

## Further comments

**Why the generated log methods change signature.** With
`[LoggerMessage]` the message
template and its parameters are a single declaration, so adding an id to
a message
necessarily adds a parameter to the generated method. Every one of them
lives in an
`internal static partial class …Log` type, so none of it is public API.

**Scope is the classic session and subscription path.**
`ManagedSession`,
`DefaultSubscriptionEngine`, `MessageProcessor`,
`Subscription/SubscriptionManager.cs` and
`Subscription/MonitoredItemManager.cs` are untouched.

**Placeholders are appended rather than woven into existing message
text** so that log
scrapers matching on the leading part of a message keep working.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-08-08 09:22:05 +02:00
Marc SchierandCopilot 2b44fcad22 Robot intent: a task-level command API with a Part 10 lifecycle (#4165) 
# Description

Implement the draft **OPC UA — Robot Intent** companion model end to
end: task-level motion verbs, a Part 10 lifecycle, a server hosting
surface, a client, two samples and a live integration suite. It replaces
the opt-in convention API merged in #4127 with an implementation
generated from a companion information model.

## Why

OPC 40010-1 *Robotics* describes robot **topology** in detail and
defines **no motion verbs at all** — its entire actuation surface is
`Start`, `Stop` and loading a named program. A conformant client can
discover everything about a robot's construction and cannot ask it to
move anywhere. #4127 filled that gap with ten verbs in an
application-owned namespace, resolved by BrowseName, and the file said
what it was: *"opt-in, explicitly non-normative industrial operation
conventions"*.

That contribution established a **vocabulary**. It established no
**lifecycle**, and the lifecycle is the harder half.

A motion takes seconds; a pick takes a minute. OPC 10000-4 §5.12.2
discards a method result when the Session ends *"independent of the task
actually performed at the Server"* — so a synchronous motion method does
not merely time out, it **loses the outcome of work that has already
physically happened**. OPC 10000-10 §4.1 gives the OPC Foundation's own
resolution: a Method performs a calculation, a **Program** runs a batch
process or a machine tool part program.

So `SubmitIntent` returns as soon as the intent is *admitted*, and what
it returns is a NodeId — an `IntentOperationState` (a Part 10 program
instance) the client subscribes to for progress and reads for the
result.

## What changed

**Model and shared code** — `Opc.Ua.RobotIntent.NodeSet2.xml` is
source-generated in `Opc.Ua.Robotics`, so the enums, the polymorphic
intent structures, `IntentOperationState : ProgramStateMachineState`,
the method states and the typed clients derive from the model rather
than being hand-written. Its only `RequiredModel` is the base UA
namespace, so a server can adopt it without OPC 40010 or DI. `PoseMath`
implements the specification's Annex C conversion between its unit
quaternion and the core `ThreeDFrame` — including the `asin` clamp that
stops a pole orientation becoming a domain error, and the
non-negative-`w` representative that makes two servers agree on four
numbers. `FrameTree` composes transforms along the frame tree.

**Verbs are a DataType hierarchy, not one Method each.** A single
submission and a mission step are then the same shape, and a new intent
is a subtype rather than a new method — which is what
`AddOperation<TRequest,TResponse>` was working around.

**Server** — `IntentControllerHost` owns admission in the
specification's order, the queue with PLCopen buffer modes, blending,
cancellation *with the server's right to refuse*, missions with a
committed base and a revisable horizon plus an IEC 61131-3 step graph,
safety-aware refusals, real-time channel brokerage, and the capability
declaration. All thirteen Methods are wired; command authority and
channel leases are released when the holding Session closes; every
client-supplied NodeId is validated against the controller being
commanded (clause 11.3). `RobotIntentNodeManager`, the fluent
`IIntentControllerBuilder` and `AddRobotIntent`/`ConfigureRobotIntent`
make it reachable, and `RobotIntentFacetCalculator` computes which of
the specification's facets an instance actually satisfies.

**Client** — `RobotIntentClient` discovers
`Server/RobotIntent/Controllers` and reads what a robot accepts;
`IntentOperationHandle` subscribes and completes on a terminal state,
recovering the result after a reconnect because clause 6.7 promises it
survives; `CommandAuthorityLease` and `RealTimeChannelLease` manage the
two exclusive resources; fluent builders cover every intent and mission.

**Samples** — all robotics samples move under `samples/Robotics/`.
`IntentEnabledRobot` is one UR5e-style offset-wrist arm on a bench,
declared completely enough to show what a conformant server publishes,
with a simulated safety source so the clause 10.4 refusals are
demonstrable. `IntentViewerClient` turns a **click on a target prim in
the OpenUSD viewport** into an intent and watches the arm execute it; it
also runs headless, which is how CI exercises it.

**Intents execute serially**, which satisfies every `BlockingMode`
constraint by construction: the specification forbids beginning a
`Single` or `Hard` intent while another executes and merely *permits*
`None` and `Soft` to overlap.

The convention API (`RoboticsOperationConventions`,
`RoboticsOperationsBuilders`, `RoboticsOperationsClient`) is removed.

### On safety — what this deliberately does *not* claim

The interface is **non-safety-rated**, and that is a property of the
technology rather than a scoping choice. OPC 10000-15 carries cyclic
safety data from a SafetyProvider to a SafetyConsumer; the consumer's
`RequestSPDU` holds an identifier, a monitoring number and one octet of
explicitly **non-safety** flags, so a caller has no channel through
which to supply safety-rated arguments. Every safety fieldbus
(PROFIsafe, CIP Safety, FSoE, openSAFETY) expresses a safety command as
a **continuously asserted cyclic signal**, because the integrity
argument rests on the fail-safe state that follows when assertion
*stops* — and a Method call has no defined behaviour when it stops being
called.

What the host does instead is **observe and refuse**:
`UpdateSafetyState` is how the application reports what the safety
system is enforcing, and admission then refuses on the same values a
client can read, so a refusal is explainable from the address space
rather than from Server-internal state. It may never *instruct* the
safety system, and no Method here commands a safe motion function,
changes an operational mode or clears a stop.

## Defects this found in the specification, fixed upstream

Implementing a specification is the only way to find out whether it can
be implemented. Seven defects went back to
[marcschier/opcua-drafts#47](https://github.com/marcschier/opcua-drafts/pull/47)
rather than being worked around here:

- **Refusal was unobservable.** Clause 6.2 orders six distinct refusals
and clause 5.8 calls the failure set "small and diagnosable on purpose"
— but `SubmitIntent` returned only `IntentId` and `Operation`, so a
client could see *that* something was refused and never *which*. It now
returns `Accepted`, `Failure` and `Message`, and clause 6.2 states that
a refusal returns `Good` rather than substituting a Bad `StatusCode`.
- **The Part 10 promotion was illegal.**
`IntentOperationType.ProgramDiagnostic` was declared as a Property of
`PropertyType`; OPC 10000-10 declares it a Variable of
`ProgramDiagnostic2Type` reached by `HasComponent`. The declaration
added a *second* member beside the inherited one instead of promoting it
— which is precisely why an earlier version of this branch had deleted
it. Correcting the TypeDefinition made it generate cleanly with **no
change to the curated Core**.
- **Clause 9 said nothing about the Method surface.** A server could
declare `MissionsSupported` true and omit `SubmitMission` entirely,
because the mission and channel Methods are Optional. This
implementation did exactly that until an integration test caught it. A
fourth honesty rule and a table now fix which Methods each declaration
implies.
- **Conformance was not machine-readable, and two implementations
disagreed.** Clause 12 defined conformance in terms of facets and gave a
Server nowhere to publish which ones it satisfied, so every client had
to re-derive Table 12.2 from the address space. This repository proved
why that fails: the server''s `RobotIntentFacetCalculator` and the
client''s `RobotIntentRules.DeriveFacets` each implemented clause 12.2
independently and **disagreed on essentially every row** — the client''s
`Blending` was `BlendingSupported` alone where the server also requires
all four blending buffer modes, its `QueuedIntents` was `MaxQueueDepth >
0` where the server also requires `Buffered`. A client and server from
the same release answered "does this controller conform?" differently,
and both were reading the specification correctly, because several rows
are behavioural and no amount of browsing settles them.
`IntentCapabilitiesType` now carries **`SupportedFacets`**, exactly as
OPC 10000-5 does with `ServerCapabilitiesType.ServerProfileArray`,
RI-Base requires it, and clause 12.2 separates structural requirements a
Server shall meet before listing a facet from attested ones governed by
clause 9.
- `WaitIntentDataType.Signal` was unbounded where clause 11.3 has to
check it; §11.3 gained a table fixing the expected type of every
NodeId-valued member; §5.7.0 gave `Ready`/`ActiveIntent`/`ActiveMission`
normative meaning; §6.9 bounded `RequestedLease`; §6.5 and §6.4 now say
what a server that cannot differentiate `StopMode` must do, and which
stop a superseded intent gets.

## Bugs the tests and reviews caught

Each of these would have been silent in the field:

- **The execution engine was never wired into the public API.**
`IntentControllerBuilder.RegisterAsync` built the address space and
never started an `IntentControllerHost`, so through the documented
`AddRobotIntent` → `ConfigureRobotIntent` path **every Method returned
`BadNotImplemented`** and the registered `IIntentExecutor` was never
resolved. A browsable address space is not a working robot interface,
and only a live integration test showed the difference.
- **NodeId scoping failed open on an empty index.** A controller
advertising `CallProgramIntentDataType` with no published `ProgramType`
accepted an *arbitrary* NodeId as `CallProgram.Program` — and
`CallProgram` runs code the server holds.
- **Blending completed the predecessor at submission time** and
abandoned its motion, telling the client it succeeded and publishing an
all-zero pose at the origin as a genuine `AchievedPose`.
- **Mission base immutability compared only identifiers**, so a client
could keep the ids and swap the *motion* of an already-committed step.
- **`FinalResultData/Result` was never registered** with the node
manager: Browse showed it, Read returned `BadNodeIdUnknown` — so clause
6.7's promise to a Part 10 client did not hold.
- `DisposeAsync` disposed primitives out from under in-flight executor
code; re-using a terminated `IntentId` destroyed the retained result and
duplicated a NodeId; neither side registered the Robot Intent
encodeables, so `Pose3DDataType` did not decode; host startup was hooked
to a task that ran in only one hosting shape.
- The result was published **after** the state went terminal;
capabilities were resolved against a stale namespace table; an empty
`ContentFilter` arrives as an empty element array rather than a null
filter, so testing only for null made every unconditional mission
transition silently untaken — a mission would run its first step and
stop.

### Found by a later test-quality audit, after the above was written

An audit of the test suite itself found five tests whose **names**
asserted a normative requirement while their **code** asserted something
weaker or nothing at all. The worst **fabricated the answer it
checked**: a helper caught a decode failure and invented "5 supported
intents, 6 axes, all facets true", so six capability-honesty assertions
across four tests could pass while reading nothing from the server. The
fallback is gone and the suite passes with honest reads.

Chasing the facet gaps it reported led to the clause 12 defect above.
Two further defects followed from completing that work:

- **`RI-Interop-40010` was never computed**, and the code documented the
omission as impossible to fix "because the calculator receives only a
Robot Intent controller, not the linked OPC 40010 model". That was
wrong: Annex B's structural requirement is an **inverse**
`HasIntentController` reference, reachable from `IntentControllerState`
— and the integration suite was already browsing exactly that. A server
implementing Annex B could not claim Annex B.
- **The published claim could go stale.** `SupportedFacets` was a
snapshot taken during `RegisterAsync`, so a reference attached
afterwards left it wrong. It is now bound through `BindRead` and
recomputed on read, so the claim tracks the address space by
construction rather than by discipline — which matters for anyone
bolting Robot Intent onto an existing OPC 40010 node manager, where the
attach ordering is often not theirs to choose.

Two tests that **could not fail** were also replaced: one asserted
`AccessLevel` equals `CurrentRead` to pin a read-only marking, but
`BaseVariableState` initialises every variable to exactly that, so
deleting the marking left it green; it is now a write expecting
`BadNotWritable`, including a real OPC UA `Write` service call over a
live session.

Finally, a build break that a plain build hides: `RoboticsAotTests` used
`out string?` in a project declaring `<Nullable>disable</Nullable>`.
Building the project **without** `-f` suppresses `CS8632` through
`NoWarn=nullable` and succeeds; building it **with** `-f net10.0`
resolves `NoWarn` differently, `CS8632` fires, and
`TreatWarningsAsErrors` makes it an error. CI builds per-TFM, so this
would have failed there while passing every local build to that point.

## Testing and coverage

| | |
|---|---|
| `dotnet build UA.slnx -t:Rebuild` | **0 errors, 2 warnings** — both
pre-existing `CA1861` in `tests/Opc.Ua.Tools.Tests`, a file this PR does
not touch |
| **Whole `UA.slnx` test suite, net10.0** | **50 of 52 assemblies fully
green** (measured at `40d9cf213`, before the `master` merge; the other
two are explained below) |
| `Opc.Ua.Robotics.Tests` | **467** net10.0, **447** net48 |
| `Opc.Ua.Robotics.Intent.Tests` (live client <-> server) | **20** on
net10.0 and net48, run repeatedly |
| `Opc.Ua.OpenUsd.Tests` | **904** after merging `master` (664 before;
the rest arrive with #4160 / #4162 / #4180) |
| `Opc.Ua.Aot.Tests` | **121/121** |
| `Opc.Ua.Di.Tests` (regression from the sample move) | **364/365**, one
pre-existing skip |
| **Changed-line coverage** | **90.7 %**, no file below 80 % —
comfortably above the enforced band, which after #4158 applies only to
patches larger than 100 changed lines |

The two non-green assemblies are **not** caused by this branch. Rather
than assert that, I ran the same selections at the merge-base
`ae241b662` on the same machine:

-
`ServiceCallReassemblerTests.PrivateRequestSummaryFormatsKnownRequestKinds`
expects `12.5` and gets `12,5` — a culture-sensitive assertion failing
on a European locale. Fails 4/4 on **both** commits.
-
`CertRotationLiveTests.L2Cert2RotateCertificateDuringServerRestartRecoversSharedChannelAsync`
fails on a teardown count. Fails 4/4 on **both** commits.
- A third failure seen once, AOT `FindServersAsync` with
`BadSecureChannelClosed`, did not reproduce: 4/4 on retest and 121/121
in the final run. Environmental.

Two things are worth calling out because they would otherwise be
invisible:

- `tools/**` is **not** in the `coverage-thresholds.json` ignore list,
and no test project referenced the OpenUSD tools assemblies — so their
changed lines counted as uncovered and the real patch figure was 75 %,
exactly the blocking threshold. Rather than widening the ignore list,
the pure decisions in the viewport host (pointer-to-physical-pixel
conversion, `RenderPickRequest` construction, stale-retry, backend
discovery, pick-mode selection) were separated from the Avalonia/native
shell and tested headless against a fake picking backend.
- `dotnet build UA.slnx` **hides warnings on unchanged projects**; only
`-t:Rebuild` surfaces them. That is how a `CA2263` introduced by a net48
portability fix survived several "zero warning" builds.

The multi-target requirement earned its keep repeatedly: it caught a
default interface implementation, `ArgumentNullException.ThrowIfNull`,
`string.Create` with an interpolated handler,
`ValueTask.FromResult`/`CompletedTask`, `Task.WaitAsync`, generic
`Enum.GetValues` and generic `Enum.IsDefined`, and a non-generic
`TaskCompletionSource` — none of which the net10.0 build complained
about. A net10.0-**only** hang in the simulated executor and two
load-sensitive flaky tests were also found and fixed at the cause.

### Merged with `master`

`master` (through `e73e71184`) is merged in as `08205d81d`. Four
conflicts, all where the two branches touched the same robot and OpenUSD
surfaces, and none resolved by dropping a side:

- #4162 added six files to `samples/MinimalRobotServer/` while this
branch moved that directory to `samples/Robotics/MinimalRobotServer/`.
They are kept at the moved path, and `UA.slnx` takes the union of both
sides' new projects.
- #4160 replaced `object` with a `UsdValue` union and rewrote `Flatten`;
this branch had reformatted the same method and fixed a path in its doc
comment. Git produced a hybrid that did not compile. The resolution
keeps `master`'s `UsdValue` arm and `Flatten(UsdValue)` overload in this
branch's block-bodied form, with the path fix retained.
- `master` inserted `FindStageCamera` directly before two methods this
branch had relocated, so the merge duplicated `WriteStageUsda` and
`GetPrivateStateRoot`. Only `FindStageCamera` is genuinely new —
`master`'s diff shows the other two as unmodified context — so the
duplicates are dropped and this branch's `internal` versions kept,
including the `GetPrivateStateRoot(string?)` overload the connector
tests use to stay out of the real user profile.
- `master` moved the OpenUSD stack to `0.4.0-alpha`; this branch pins
`OpenUsd.Rendering` in the test project so the renderer-pick tests can
reach the picking backend. Left at `0.1.0-alpha` that pin is below what
the viewer now resolves transitively, which is `CS1705`. The pin follows
`master`.

CI was green on `baaf35e9` with **190 checks passing and none failing**,
across every build leg (net472 / net48 / netstandard2.0 / netstandard2.1
/ net8.0 / net9.0 / net10.0, Debug and Release) and the full test matrix
on Windows and Linux. Since then `master` has been merged twice more and
two CI-stability fixes landed (see below), so the current head is
re-running. Locally, `dotnet build UA.slnx -t:Rebuild` gives **0
errors** and the 2 pre-existing `CA1861` warnings; Robotics **467**
net10.0 / **447** net48, Robot Intent **20** on both, OpenUSD **904**,
AOT **121/121**, source generation **3771**.

### Review round

Five reviewers over disjoint areas found defects worth calling out, all
fixed in `3ffec2c1`:

- **A refused mission step was reported `Succeeded`.** The advance path
returned the same `false` for "steps exhausted" and "step refused", so a
protective stop between steps told a supervisory system the mission had
completed while the robot stood halted with work unexecuted.
- **Three attacker-controlled NodeIds reached the executor unvalidated**
— `Pick.Tool`, `Place.Tool`, `ToolChange.DockStation`. `DockStation`
drives a physical tool-exchange motion to a location the server never
authorised. The replacement test enumerates NodeId members by reflection
so the next one added cannot be missed.
- **A lock-order inversion that loses a stop.** `CanCancel` and the
mission condition evaluator ran under the host lock while the executor's
own thread re-entered it via `ReportBlendBegin`.
- **A non-terminal executor outcome killed the pump** from outside the
`try/catch` that exists because the executor is untrusted — `Ready`
stayed true and submissions kept being accepted while nothing moved.
- **`Pause` published `Suspended` while the robot kept moving.** Now
queue-only and honest about it; the specification gained the matching
rule ([opcua-drafts
`357a558`](https://github.com/marcschier/opcua-drafts/pull/47)).
- **`RI-RealTimeChannel` and `RI-Safety` were false claims** — granted
on an empty folder with no methods, and on the mere presence of a
Mandatory node.
- **The Part 10 promotions never promoted.** Namespace-1 BrowseNames
declared second members beside the inherited ones; the conformance test
could not see it because its lookup stripped the prefix. Fixed upstream
and regenerated.
- **Renderer picking never worked.** The reflective probe bound a
renderer that is not a picking backend and throws off its owner thread.
It now binds only `IRenderPickingBackend`, falls back immediately and
loudly, and the docs say plainly that renderer picking is unavailable
with this package version.
- **The client lost command authority the instant it gained it** —
monitored items report the current value first, and the lease treated
every notification as a loss.

### CI stability fixes for master-side failures

The CI split in #4158 turned on legs that had never run, and two of the
failures they exposed were real bugs on `master` rather than anything
this branch introduced. Both are fixed here because they were blocking
every branch:

- **ECC key generation was broken on macOS.** `ExportExplicitParameters`
throws `PlatformNotSupportedException` there — Apple's
SecurityTransforms supports named curves only — and it was called for
exactly one value, the curve order `n`. The four curves OPC 10000-12
admits have published orders, so those are carried directly, with
`CurveOrderTableMatchesThePlatform` checking each against the platform's
own exported order wherever export works, so the constants cannot
silently drift. This accounts for the two ECC failures in
`test-macOS-latest-Server`.
- **The certificate-leak detector raced asynchronous disposal.** It
polled a fixed budget and reported whatever it saw, which cannot
distinguish a genuine leak (count steady) from a suite still draining
fire-and-forget disposals (count falling) — on a loaded agent that
produced `Certificate leak detected: 2 instance(s)` with every test
passing. It now waits for the count to **settle**, so a real leak is
reported sooner than before while a slow drain gets as long as it keeps
progressing. Shared by every suite that asserts no certificate leaks.

The remaining macOS failures on `master` (`Gds`, `Tools`,
`Redundancy.Samples`, `aot-macos`) have different root causes, are not
reproducible without a macOS host, and are **not** addressed here.

### Merged with `master` again

`master` through `416d619c4` (#4158, the CI matrix split and graduated
coverage bands) is merged in as `08ae4528`. One conflict, in
`docs/DeveloperGuide.md`, where both sides had deliberately rewritten
the coverage-gate section:

- `master`'s rewrite is kept in full — graduated `patch.bands`, Codecov
being informational-only, and where the numbers surface per CI system.
- This branch's paragraph about the `ignore` globs is kept too, re-sited
ahead of those subsections. I re-checked it against the merged
`coverage-thresholds.json` rather than assuming it still held: the list
is still `tests/**`, `samples/**`, `obj`, `bin`, `*.g.cs`, with
**`tools/**` absent** — so the warning that anything under `tools/` is
measured like product code, and that an assembly no test project
references contributes changed lines counted as *uncovered*, is still
accurate and worth keeping.
- `master`'s "reproduce a coverage failure locally" line supersedes this
branch's near-identical one, so only `master`'s survives.

Post-merge: `dotnet build UA.slnx -t:Rebuild` **0 errors**, 2
pre-existing `CA1861`; Robotics **467** net10.0 / **447** net48; Robot
Intent **20** on both.

### Getting CI green

The pipeline had never actually run on this branch until now (it was
waiting on the fork-PR approval gate), so the first run surfaced four
real problems, each fixed at source:

- **Seven build legs failed** because `IntentEnabledRobot` pins
`net10.0` for the OpenUSD payload but never opted into
`RestrictForLegacyTfm`. On the legacy legs the libraries build as
net472/net48, where `Opc.Ua.Types` exposes the `System.Threading.Lock`
polyfill publicly, and that collides with the sample's own net10.0 BCL
(`CS0433`). `Opc.Ua.OpenUsd.Connector.Viewer` was already using the
opt-out for the same reason.
- **A Linux-only test failure** turned out to be a real defect, not a
platform quirk: `SetPose` published the *commanded* interpolated pose
beside joint frames derived from the *solved* joints, so the two
disagreed by the inverse-kinematics residual — about 8 mm, constant
after any real motion. That is the lying-digital-twin defect one level
deeper than the per-step IK fix reached. `SetPose` now derives both from
the joint angles.
- **Three source-generator tests** asserted the old emitted shape. The
clause 6.2 Null-Variant fix was implemented in
`ObjectTypeProxyGenerator` so every generated proxy benefits, which
changed that shape; the tests are updated and strengthened to pin the
`IsNull` branch as well.
- **A CI-only timeout** in the simulated arm drain: the executor runs on
a manual clock so the tests do not depend on machine speed, but the
drain guarded itself with a wall-clock deadline, which put that
dependence back. CI runs with coverage instrumentation, which is enough
to take a per-step IK probe from ~0.6 s to over 5 s. The guard is now a
simulated step budget.

One failure was **not** from this PR and did not recur: a Roslyn
`AccessViolationException` compiling `Opc.Ua.ISA95` on one leg, while
the Debug leg of the same target passed on identical code and the
Release build succeeds locally.

## Related Issues

- Relates to #3827 — the request that prompted the robotics intent work.
- Builds on #4127.

The companion specification is drafted in the open at
[marcschier/opcua-drafts#47](https://github.com/marcschier/opcua-drafts/pull/47)
(`metaverse-specs/robot-intent/`), with the prior art and the reasoning
behind each decision in the research document beside it. **Nothing in it
is normative or endorsed by the OPC Foundation, and its NodeIds and
namespace URI are provisional** — which is the main reason this is a
draft.

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

On the test checkbox, precisely: the whole `UA.slnx` suite now runs on
net10.0 with **50 of 52 assemblies fully green**, and
`Opc.Ua.Robotics.Tests` passes on **all five** target frameworks
including net472, net8.0 and net9.0 which had never previously been
exercised. The two remaining failures are reproduced identically at the
merge-base and are detailed above, so I have ticked the box; if you
would rather it stayed unticked until those two are green, say so and I
will revert it.

## Points I would most like feedback on

1. **The namespace and NodeIds are provisional.** They come from a draft
specification, not from the OPC Foundation. If this lands before the
model is registered, the identifiers will move.
2. **The model lives in `Opc.Ua.Robotics`.** It is standalone on the
base UA namespace and takes no dependency on OPC 40010, so a separate
`Opc.Ua.RobotIntent` package would arguably be cleaner. Keeping it here
was a deliberate call — this is where the robot-facing API already is —
but it is the easiest thing to change now and the hardest later.
3. **Removing the convention API from #4127 is a breaking change** for
anyone who adopted it. It was explicitly marked non-normative, so I have
assumed that is acceptable — please say if a deprecation period is
wanted instead.
4. **Serial execution** is a deliberate simplification. Concurrent
`None`/`Soft` intents would be an optimisation, not a correction, but it
is worth agreeing that reading.
5. **`samples/Robotics/` reorganisation** moves `MinimalRobotServer`.
Happy to split that into its own commit if it makes review easier.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
 2026-08-08 09:08:40 +02:00
Marc SchierandCopilot fb80bfdc82 [WotCon] Track WoT Connectivity/Binding 1.1 and add projections (Section 12) (#4182) 
Follow-up to #4128, which merged the WoT Connectivity 1.1 work against
**draft2** of the companion specifications. Both drafts have moved
since, and this branch tracks them through to **draft3**, adds the
projection feature they introduced, and closes the gaps a bidirectional
re-audit found.

The specifications are the private
[`OPCF-Members/spec-drafts`](https://github.com/OPCF-Members/spec-drafts)
drafts of *OPC UA — WoT Connectivity 1.1* and *OPC UA — WoT Binding
1.1*. They are still drafts, which is why this is opened as a draft PR —
see [Status](#status) at the bottom.

## What changed in the specifications

Two revisions landed upstream while #4128 was in flight.

**draft2** replaced the `eventGroup` / `actionGroup` grouping constructs
with a single **projection / view** concept, added fifteen Section 6
vocabulary terms, and made two identifier rules normative. **draft3**
([spec PR #8](https://github.com/OPCF-Members/spec-drafts/pull/8))
un-deprecated the incorporated OPC 10100-1 v1.02 surface, restructured
the conformance clause into three profiles, and tightened two security
obligations.

The draft3 NodeSet is **structurally identical** to draft2 — I diffed
all 286 nodes node-by-node. Every change is metadata: 96
`ReleaseStatus="Deprecated"` attributes removed, `PublicationDate`
advanced, and `WOTC-ProjectionMaterialization` claimed by three Type
Nodes that previously implemented it in name only.

## Projections (WoT Binding Section 12)

The substantial addition. A projection document selects affordances from
other Thing Descriptions or Thing Models and resolves to a document of
its own; materialized, it becomes an OPC UA `View` that **creates no
affordance Node** and only `Organizes` what its sources already
materialized.

| Piece | Where |
|---|---|
| Document model and validation | `Opc.Ua.Types/Wot/WotProjection.cs`,
`WotProjectionModel.cs` |
| Four-stage resolution engine |
`Opc.Ua.Types/Wot/WotProjectionResolver.cs` |
| Materialization plan |
`Opc.Ua.WotCon.Server/Materialization/WotViewProjectionPlan.cs`,
`WotProjectionViewBuilder.cs` |
| Live address-space host | `WotProjectionViewNodeManager.cs`,
`LifecycleWotViewProjectionHost.cs`, `IWotViewProjectionHost.cs` |
| Node lookup seam | `IWotMaterializedNodeIndex.cs` |

Groups reached by `ua:Organizes` become organizational Objects rather
than nested Views, only the outermost materialization is a `View`, and a
member whose source is served by another server is omitted and reported
rather than recreated — the federation case the clause calls out.

### How this is validated

The specification publishes twenty worked examples, two of which are a
**golden pair**: a projection document and the resolved view it is
defined to produce. `WotSpecExampleTests` embeds all twenty and runs the
pair through the resolver, asserting against *the specification's own
expected output* rather than against my reading of the prose. That
single document exercises all three selection forms, the bulk naming
rule, the security closure naming and the provenance term.

This is the strongest evidence in the PR, and it is worth more than the
unit tests around it: it is the same check the upstream
`validate_local.py` performs.

## Other spec work

- **Section 6 vocabulary** — fifteen missing model and platform terms
with their validation rules (`WotNodeSetConverter.ModelVocabulary.cs`).
- **Portable identifiers** — the two rules release 1.1 made normative,
with `WotNodeSetConverterOptions.AllowNonPortableIdentifiers` as a
documented downgrade-to-warning switch for documents authored against
v1.00. Defaults to `false`.
- **xRegistry §6.9 identifier construction** (`XRegistryIdentifier.cs`)
— reverse-authority symbolic identifiers; all seven published worked
examples are test cases.

## Gaps found by re-auditing against draft3

I audited in both directions — every normative requirement to code, and
every `uav:` term in code back to a spec that defines it. Three
findings, all fixed:

1. **`Wot-Con 1.02` accepted anything well-formed.** draft3 makes format
validation part of the unit, because *WoT-Con Minimal* is one unit wide
and materialization projects a *valid* document. The 1.02 upload path
only deserialized into the `ThingDescription` POCO, which happily
accepts JSON that is not a WoT document at all — every member of that
type is optional, so `{}` deserialized and was materialized. It now
parses via `WotDocument.Parse` and requires the document to identify
itself by a non-empty `name` or `title`, returning `Bad_DecodingError`
and materializing nothing otherwise. The rule lives in
`ThingDescriptionFormatValidator` and is shared with
`CreateAssetForEndpoint`, whose auto-generated Thing Descriptions §11
also treats as untrusted input.
2. **Two `WoTFile` operations were ungated.** draft3 §11 names `Write`
and `CloseAndUpdate` alongside the five management methods; both reached
the materializer without an access check. `EnforceManagementAccess` is
now wired into `WotAssetFileManager`.
3. **Two invented `uav:` terms.** `uav:NodeSet2Preservation` was
declared and never used — removed. `uav:eventFields` is read by
`OpcUaBindingPlanner` and defined in *neither* spec, while a comment
claimed it came from OPC 10101. It still works for compatibility but is
now documented as a non-standard extension at both sites rather than
misattributed.

The SSRF prohibition is satisfied by construction: there is no HTTP
client anywhere in these assemblies, and all resolution goes through the
injected `IWotThingResolver`.

## Two ambiguities fixed in the specifications themselves

The audit turned up two places where a conforming implementer had to
invent an answer. Rather than pick silently, I took both upstream — they
are pushed to [spec-drafts PR
#8](https://github.com/OPCF-Members/spec-drafts/pull/8) (all checks
green) and are now normative in *Binding* §12.6 and *Connectivity*
§7.13:

- **`ViewVersion`** had one sentence saying only *that* it changes. It
now changes iff the resolved membership changes, is updated at commit
time alongside the `NodeVersion` stamping, and is a deterministic
function of the membership alone in a canonical order — so two servers
that resolved the same membership agree, which a per-server counter
would not across a redundant pair. Explicitly **not monotonic**: compare
for inequality only.
- **"Type-level view"** was used once in §12.2 and defined nowhere,
while every other statement of materialization was universal. §12.6 now
*defines* it rather than contradicting §12.2: the NodeClass is `View` in
both cases.

Both were already what this code did, so neither changed behaviour — but
the order-insensitivity the clause added was **not** covered by any
test, and `ViewVersionIsUnchangedWhenOnlyTheOrderOfTheMembershipChanges`
now holds it. It is mutation-verified: removing the ordinal sort turns
it red.

`docs/WotBindings.md` no longer lists these as points "the specification
leaves open".

## Validation

| Suite | Result |
|---|---|
| `Opc.Ua.Types.Tests` | 8525 ✅ (8518 on net48) |
| `Opc.Ua.WotCon.Tests` | 1057 ✅ (net10.0 and net48) |
| `Opc.Ua.WotCon.Bindings.Tests` | 559 ✅ |
| `Opc.Ua.XRegistry.Tests` | 196 ✅ |
| `Opc.Ua.Server.Tests` | 4027 ✅ |
| `Opc.Ua.SourceGeneration.Core.Tests` | 3771 ✅ |
| `Opc.Ua.WotCon.Samples.Tests` | 9 ✅ |

The full `UA.slnx` builds with **0 errors and 0 warnings attributable to
this PR** on all 6 TFMs (`TreatWarningsAsErrors` is on).
`Opc.Ua.Server.Tests` and the source-generation suites are listed
because this PR now also touches `MasterNodeManager` and the WoT-Con
NodeSet.

## Compatibility

No public API is removed or broken. Everything added is new surface
using `ArrayOf<T>` / `ByteString` per the repo conventions, wired for DI
with a direct-construction fallback, and the one behavioural tightening
(`Wot-Con 1.02` format validation) rejects input that was never valid to
begin with.

One pre-existing issue is **not** addressed here because it came in with
#4128 and is out of scope: `WotAssetFileManager.OnCloseAndUpdate` uses
`.GetAwaiter().GetResult()`, which the repo rules ban. Worth a
follow-up.

## Landed since this PR was opened

- **DI upgraded to 1.05.0.** `ConnectsTo` was declared a subtype of
`HierarchicalReferences`, contradicting OPC 10000-100 §5.5 Table 48.
That is an upstream error in every official DI NodeSet up to 1.04,
corrected by the OPC Foundation in 1.05.0, so the fixture is replaced
with the verbatim 1.05.0 file rather than hand-patched. A guard test
pins the corrected form. This also closed a second gap: six DI types the
WoT model was missing are 1.05 additions.
- **The sample Pump has the type definition OPC 40223 declares for it.**
`Pump1.Identification` carried DI's `FunctionalGroupType` instead of
`PumpIdentificationType`. Every test, the sample documents and the
README agreed with each other about it, which is why it survived.
- **`WotPumpAddressSpaceComparisonTests`** compares this server against
`PumpDeviceIntegrationServer`, which builds the same OPC 40223 Pump from
generated NodeSets and hand-written wiring. An independent oracle rather
than a restatement of the same assumptions - it is what found the defect
above.
- **Error containment in `MaterializeProjectionViewsAsync`.** An
exception mid-loop discarded the applied View handles before they were
published, leaving View Nodes in the address space that nothing could
remove.
- **`HasWoTProjection`** is now created; it was documented and plumbed
but never wired.
- **The default in-memory View host** no longer deletes the View it has
just applied.
- **An undeclared NodeSet alias now says so.** It previously surfaced as
`IdentifierMissing`, naming neither the value nor the fact that an alias
was expected.
- **`ViewVersion` follows the algorithm the specification now states.**
My earlier spec text was rejected on review, correctly: it named neither
the function nor the canonical order it demanded, ignored that OPC
10000-3 §5.4 requires a value greater than zero, and its "when, and only
when" is unsatisfiable for a 32-bit value. §12.6 now specifies the
algorithm outright and this implementation follows it exactly, pinned by
an independently computed test vector. Implementing it surfaced a
further defect in the clause - joining members on U+000A is not
injective, because a NodeId string identifier may itself contain U+000A
- which is fixed in the specification by length-prefixing each member.
- **An auto-generated Thing Description is treated as untrusted input.**
§11 requires it; `CreateAssetForEndpoint` validated the endpoint but
handed whatever the discovery provider returned straight to the
materializer. The `Wot-Con 1.02` format rule now gates both paths from
one place.
- **Document bytes moved out of the registry snapshot** into a
content-addressed store, so a commit no longer rewrites the whole corpus
to edit one document. Writes stage and are promoted during the commit,
which preserves both durability-first ordering and the fail-closed rule
that a blob directory without a manifest means operator recovery.
Restoring streaming digest verification along the way closed a real gap:
content integrity had degenerated to a length comparison.
- **External references now reach a NodeManager registered later.** A
reference to a Node that does not exist yet is dropped rather than
queued, and the dynamic-registration path had no second phase to apply
them in, so registering a NodeSet before the one it points into lost the
forward edge permanently. Review of that change then found a state leak
in its rollback path, fixed with a red-to-green regression test.
- **`WOTC-Legacy` is renamed `Wot-Con 1.02`** in the specification and
here. Editorial: the NodeSet diff is `<Category>` values only.

## Status

Open for review. Two things may still move:

1. **The specifications are unpublished drafts.** Anything here can
change if the working group decides differently. The `ViewVersion`
definition has since been settled - §12.6 specifies the algorithm and
this follows it - but the same could happen to any other clause tracked
here.
2. **`uav:eventFields`** is kept as a documented non-standard extension.
That is a judgement call the WG may want to reverse, either by adopting
the term or removing it.

Two known items are recorded rather than fixed:

- `Pump1` has no hierarchical parent, so it is reachable by NodeId but
not by browsing down from `Objects`. The cross-NodeSet ordering defect
this was originally attributed to is fixed above, and cross-manager
references are now symmetric in both registration orders. The Pump
itself is still blocked on a different cause found while verifying that
fix: with the reference added, `DeviceSet` exists but the Pump has
**zero** inverse references, so the edge is dropped at *import* - before
any external-reference machinery runs. That is a separate defect in the
materialization path, not the one that was fixed, and it is left
recorded rather than guessed at.
- `WotAssetFileManager.OnCloseAndUpdate` uses
`.GetAwaiter().GetResult()`, which the repo rules ban. Pre-existing from
#4128 and out of scope here.

Review of the projection resolver and the materialization host is the
most valuable thing to look at; the vocabulary and identifier commits
are mechanical by comparison.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
 2026-08-08 09:08:23 +02:00
Marc SchierandCopilot ca89ac85a7 Add a Generators companion-spec sample and compose servers at a site level (#4180) 
# Description

Adds a **Generators companion-specification sample** alongside the
existing pump sample, and a **`SiteCompositionServer`** that composes
several device servers into one OpenUSD scene at a supervisory level —
demonstrating that servers can be aggregated without mirroring their
address spaces.

Three things, in dependency order:

### 1. `GeneratorServer`

A self-contained server realising the draft Generators spec
(`http://opcfoundation.org/UA/Generators/`) end to end: N simulated
generating sets, DI + Machinery integration, a twelve-state operating
state machine, four protection alarms per set, six control methods, and
a per-set OpenUSD twin.

The organising idea is that **load fraction is the only independent
variable**. Everything else is a function of it:

```
V̇_f(x) = 3.67 + 100·x                fuel rate      [L/h]
η(x)   = P(x) / (V̇_f(x) · ρ · LHV)   efficiency
S = P / PF     I = S / (√3·V_LL)     f = N·p / 120
```

This is not stylistic. When each measurement is an independent
oscillator — as the pump sample's simulation once was — a server happily
publishes a duty point no real machine could occupy, and no test can
catch it because there is nothing for the values to be inconsistent
*with*. Deriving them from one variable means `P = √3·V·I·PF` and `η =
P/(V̇·ρ·LHV)` hold at every tick by construction. The published
`DATASHEET.md`, the engineering ranges, the trip points and the
simulation all read the same constants in `GeneratorDatasheet.cs`, so
the document and the server cannot describe different machines.

The machine is a fictitious **SimGen Systems GenX-500** (400 kW prime /
440 kW standby, 400/230 V, 50 Hz, four-pole). Figures are representative
of a real industrial genset class; no vendor is named anywhere in the
code or docs.

### 2. Cross-server composition in the connector

`RemoteSessionFactory` already existed in the client library but the
connector CLI never set it, so cross-server composition was unreachable.
`--federate` wires it up: the connector opens a session to each server
named by a component binding, discovers its representations and drives
its bindings into the same stage.

It is **opt-in** because the endpoint the connector dials comes from the
server being rendered rather than from the operator, which makes
honouring it a trust decision. Federation is best-effort per component —
a subordinate that is down is logged and skipped, and the rest of the
scene still renders — which is the only `src/` change in this PR.

### 3. `SiteCompositionServer`

Owns no devices. It publishes a site stage plus one cross-server
component binding per subordinate, carrying that server's
`ComponentServerUri` and `ComponentEndpointUrl`. Nothing is mirrored, so
there is no cache to invalidate and no second copy of the truth.

## Notes for reviewers

**Everything interesting in this PR was found by running the servers,
not by reading the code.** Six defects, none of which looked wrong in
review:

- `IsShutdown` and `SubsystemName` are *optional* on
`GeneratorProtectionAlarmType`, so the generated factory does not
materialise them. `CreateOrReplace` alone produces a child that exists,
appears in `GetChildren` and holds the right value — but carries **no
`ReferenceTypeId`**, so no browse can reach it. Every alarm was
publishing its trip without saying whether the trip stops the machine or
which subsystem to go to. `AddXxx(context)` first is what gives the
child its `HasProperty` reference.
- **The protections could never fire.** Every trip point sits outside
the band the datasheet curves produce — that is what a datasheet *means*
— so all four alarms, the shutdown class, `ResetFaults` and the whole
`Fault` branch of the state machine were unreachable while the README
documented them as observable. Overload was worse: the load clamp
ceiling was set to exactly the trip point, making a
strictly-greater-than comparison unsatisfiable. Faults are now
injectable, and the first set — the one the hero camera frames —
develops one on a slow rotation.
- **A shutdown trip removes the condition that caused it**, because oil
pressure and coolant temperature are only supervised while the engine
turns. Each alarm went active for one tick and cleared, leaving an
operator with a stopped machine and no indication of why. Shutdown-class
alarms now latch until the set leaves the shutdown state. Every unit
test that checked the *condition* passed throughout — the defect only
appears when the trip and the supervision interact over time.
- **`Synchronizing` and `Paralleled` were declared, drawn in the README
and never entered.** The first set now energises a dead bus and closes
onto it while every other set synchronises to a live one — both how a
real plant parallels, and what makes the two states observable.
- **The tick raced client method calls.** The simulation tick runs on a
thread-pool thread while method calls arrive on request threads, and
both transition sets and write the same nodes. A tick moving a set to
`Cooldown` and a concurrent `EmergencyStop` could interleave the paired
`CurrentState` / `CurrentState.Id` writes and leave a client with a
state *name* from one transition and a state *node* from the other — the
exact failure the paired write exists to prevent, reintroduced by the
threading model. Both paths now take one gate.
- Low oil pressure supervised from raw speed **tripped every set during
cranking**, because pressure has not built yet. Now gated on
`IsSpinning`, which is what a real start-up bypass does.

Smaller fixes: shutdown trips are applied after the whole evaluation
pass (stopping mid-loop made the remaining conditions read healthy,
collapsing simultaneous trips to whichever came first in the table);
`ResetFaults` reports its clears as events (a client learns of condition
state changes only through events, so a silent clear leaves an
alarm-list client showing it forever); the start counter moved into the
transition so commanded starts are counted; the federated connector
closes the remote session if the connector constructor throws between
taking ownership and registering it; and `prepare_machinery_nodeset.py`
now *asserts* the IA namespace is last before removing it rather than
only claiming so in its docstring — removing any earlier entry would
silently renumber every namespace after it and rebind their NodeIds.

**A model limitation is left visible rather than papered over.**
`GeneratorStateMachineType` declares an emergency stop only out of
`Running`, `Loaded` and `Paralleled`, so the sample refuses
`EmergencyStop` from `Starting` and `Warmup`. A real panel stops from
anywhere. That is the specification's shape, not this sample's choice,
so it is pinned by a test and named in the docs.

**`Model/` vendors a reduced Machinery nodeset**, derived mechanically
by whitelist. The full official nodeset does not survive the model
source generator (`MODELGEN003`) and drags in IA through a single
optional `Stacklight` member a generating set does not have. Deriving it
by script keeps the provenance checkable; the whitelist is the only
thing to edit when more types are needed. **Never hand-edit the
generated output.**

**The federated scene renders end to end.** Getting there surfaced three
more defects, all of which produced a plausible-looking scene with
nothing in it: the generator sample's plant aggregation was created
under an already-registered `DeviceSet` and so was invisible to every
client (it had *never* rendered geometry, standalone or federated); the
connector's asset fetch ran on the primary session only, so composition
referenced layers that were never downloaded; and both generator colour
bindings wrote `primvars:displayColor` to prims that did not declare it,
so the renderer rejected every update while the file still looked
correct. Cross-server components compose under the subordinate's own
root, so the site layer now *places* `/Plant` and `/Powerhouse` rather
than leaving them stacked on the origin.

**`OpenUsdRepresentation` is now mounted with `HasAddIn`, not
`HasComponent`** (review feedback). The nodeset says so explicitly —
*"Mounted with HasAddIn"* — but every server in the tree used plain
`HasComponent`. It survived because `HasAddIn` is a *subtype* of
`HasComponent`: the representation still browses, still aggregates and
still drives a twin, so every functional test passed before the fix and
after it, and only a conformance checker could tell. Seven mount sites
corrected. `PumpDeviceIntegrationServer` and `MinimalRobotServer` had
hand-rolled the mount instead of calling the shared
`CreateRepresentation` helper, which is exactly how they drifted from
it. Both sample E2E suites now assert the reference type across
**every** discovered representation — a narrower check would have missed
the plant-level representation that arrived later from master still
carrying `HasComponent`.

**Live colour is not achievable in the current OpenUSD viewer, and that
is now documented rather than worked around.** Proven with a standalone
probe: `primvars:displayColor` resolves as `color3f[]` from the
`UsdGeomGprim` schema whatever the layer declares, and the `OpenUsd
0.4.0-alpha` managed API has no writer for that type. Bound
`UsdPreviewSurface` materials are not shaded either, so geometry renders
grey unless it also carries an explicit `displayColor` primvar. The
**visibility** bindings carry no such caveat and are what the samples
rely on to show state. Filed upstream as
[openusd-dotnet#2](https://github.com/marcschier/openusd-dotnet/issues/2),
[#3](https://github.com/marcschier/openusd-dotnet/issues/3) and
[#4](https://github.com/marcschier/openusd-dotnet/issues/4), and linked
from `docs/OpenUsd.md` so a reader can check whether they have since
been fixed.

## Related Issues

No tracking issue — this is additive sample and documentation work with
a single small, self-contained change to `src/` (per-component error
isolation and session-ownership hardening in
`OpenUsdConnector.Composition.cs`). Happy to open one if maintainers
would prefer it tracked.

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

### Testing

The generator fixtures live in `tests/Opc.Ua.OpenUsd.Tests/Generator` —
**82 tests**, no new project. `Opc.Ua.Di.Tests` would have been the
natural home next to the pump fixtures, but both device samples
source-generate their own reduced `Opc.Ua.Machinery` model and expose it
via `InternalsVisibleTo`, so referencing both from one assembly makes
every Machinery type ambiguous (CS0433); `Opc.Ua.OpenUsd.Tests`
references no other sample and already hosts `RobotAssetContractTests`.
The suite runs **870** on net10.0 and net48 (784 existing + 86) and
**788** on net472, where the sample does not exist and only the
framework-independent asset-contract tests run — forgetting exactly that
exclusion broke CI on the sibling pump PR.

The tests target the things in this sample that fail *quietly*:

- **Datasheet conformance** holds the model to its own claim — rated
speed is `120·f/p`, rated current follows from the rating, the fuel
curve reproduces the published table, and `η = P/(V̇·ρ·LHV)` reconciles
at every load. Efficiency is swept across the whole simulated range to
show it stays inside (0,100) and rises monotonically; a fit that goes
negative somewhere in its range is a coincidence that happens to look
right at the duty point, not a model. A further test reads
`DATASHEET.md` and checks the document quotes the figures the server
actually serves.
- **Drift between the two descriptions of the state machine** — the
physics' `IsLegalTransition` and the model's `GeneratorStateMap` — is
checked in *both* directions. A transition the physics permits but the
map lacks moves a machine without telling a client; one the map holds
but the physics refuses is dead weight that looks supported.
- **Reachability at run time**, not just in the declared table — the
test that catches states the model declares and nothing ever enters.
- **That every protection can actually fire**, which is the half of the
alarm contract that "a healthy set annunciates nothing" does not cover.

A **hosted end-to-end fixture** connects a real client and asserts what
the address space actually exposes — the aggregation at
`/Powerhouse/Generators`, its component binding, and every per-set twin.
That is the only kind of test that catches the registration defect
above: the node object had the right browse name, binding and asset
reference, so every test that inspected objects passed while no client
could see it.

**Every state-machine and protection fix was checked by mutation.**
Restoring the clamp ceiling, the direct `Running → Loaded` path, the old
start-counter placement, or removing the alarm opt-in each makes the
corresponding test fail, naming the specific defect. A test that has
never been seen to fail has not been shown to work.

Verified against a running plant: `Synchronizing`, `Paralleled` and
`Fault` all observed on a two-set server, with
`HighCoolantTemperatureAlarm` seen active on the fault subject. Method
behaviour verified separately — per-set distinct `CurrentState` nodes,
`EmergencyStop` → `EmergencyStopped` with `Start` then refused as
`BadInvalidState`, `ResetFaults` → `Off`, `SetOperatingMode(9999)`
refused with the mode unchanged, and the other sets untouched
throughout. Also verified as a three-server federated stage.

0 warnings on a clean Release rebuild of both samples, the client
library, the connector and the tests, across every TFM.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe608993-f3c9-4779-a6b9-9a9eabfc24ba
 2026-08-07 10:48:02 +02:00
Marc SchierandCopilot 416d619c41 Split the CI matrix across both systems, split the required test gate from the advisory coverage check, and give background work an owner (#4158) 
# Description

Two changes to how CI is routed and how it reports.

## 1. Split the CI matrix back across both systems

Flips both halves of the CI backend switch from `ado` to `actions`, so
the workload is shared instead of concentrated on the managed pool. The
switch is checked into source in **two places that must always be
flipped together**:

| File | Setting | Before | After |
| --- | --- | --- | --- |
| `azure-pipelines.yml` | `parameters.ciBuildBackend` | `ado` |
`actions` |
| `.github/workflows/buildandtest.yml` | `env.CI_BUILD_BACKEND` | `ado`
| `actions` |

| Work | GitHub Actions | Azure Pipelines (managed pool) |
| --- | --- | --- |
| All-TFM solution builds | `build-all-tfm-windows` + `-linux` | two-TFM
PR gate (`net10.0`, `net48`) |
| Test matrix | ubuntu, every test project | Windows `net48` + `net10.0`
fast PR legs |
| Native AoT | ubuntu | Schedule/Manual only |
| macOS | always (no managed-pool macOS image) | — |

## 2. Split the required gate from the advisory coverage check

The coverage gate job was appended to *every* test stage, so `Fast PR
test` emitted three sibling checks whose display names collided —
`Coverage Release (net10.0)` appeared twice, once for the Windows leg
and once for macOS, with nothing to tell them apart:

```text
✓ Coverage Release (net48)      55s
● Coverage Release (net10.0)
○ Coverage Release (net10.0)
```

That job also fused two unrelated concerns: whether the tests passed
(which must block a merge) and whether coverage met its thresholds
(which should not — the `net10.0` leg had `enforceCoverage: true`, so a
coverage dip blocked merges).

Both concerns now get a stage of their own, mirrored in both CI systems:

| Concern | Azure Pipelines | GitHub Actions | In the branch ruleset? |
| --- | --- | --- | --- |
| Every test passed | **`Tests passed`** stage | **`build-and-test
summary`** job | **Yes — required** |
| Coverage meets thresholds | **`Code coverage`** stage | **`code
coverage`** job | **No — advisory** |

The coverage check reports a clean failure when the thresholds are
missed, so the miss is visible, but it never blocks the merge.

Coverage is now merged **once** across every leg of the run instead of
once per leg, which also removes the `enforceCoverage` /
`publishCoverage` "exactly one invocation may set this" parameters.
Neither check re-runs any tests — the previous design re-ran the whole
suite in one job with the collector attached, which serialised a suite
that is deliberately fanned out and always exceeded the stage timeout.

### Coverage checks now exist in GitHub Actions

`build-and-test` already ran with `--collect:"XPlat Code Coverage"` and
uploaded the fragments, but nothing ever merged or evaluated them. It
now runs the same `check-coverage.ps1` against the same
`coverage-thresholds.json` as Azure. Since `CI_BUILD_BACKEND` defaults
to `actions`, that ubuntu matrix — which runs *every* test project — is
the fullest coverage signal in the repository.

### Reporting the numbers

`check-coverage.ps1` gained `-SummaryPath`, rendering a markdown report
that both systems surface:

- **GitHub Actions** — appended to the job summary and upserted as a
single **sticky pull-request comment**, updated in place on each run.
Threshold misses also appear as `::error::` annotations.
- **Azure Pipelines** — attached to the build summary via
`##vso[task.uploadsummary]`.

The report is a pass/fail table of the project floor, patch coverage and
baseline delta, with the uncovered changed lines listed by file in a
collapsible block. Rendered example:

> ## Code coverage
>
> ❌ **Coverage gate failed.** This check is advisory and does not
block the merge.
>
> | Check | Result | Threshold |
> | --- | --- | --- |
> | ✅ Project line rate | **72.10%** (40 731/56 496
lines) | >= 70.00% |
> | ❌ Patch coverage | **0.00%** (0/1 changed lines) | >= 75.00% |

The comment is fork-safe: a `pull_request` from a fork gets a read-only
token regardless of the `permissions:` block, so it degrades to the job
summary alone. The sticky comment uses `actions/github-script` rather
than a community action, keeping this workflow on first-party actions
only.

## Notable correctness details

These were found and fixed during review, and are the parts most worth a
second pair of eyes:

- **`dependencies.<stage>.result` resolves at *stage* scope only.** At
job scope `dependencies` refers to other jobs of the same stage, so
reading stage results from a jobs template would have silently returned
empty and the gate would have passed everything. `gate.yml` and
`coverage.yml` are therefore **stage** templates that derive their own
`dependsOn` from the same stage list they evaluate, so the two can never
drift apart. An unresolved result is a hard error.
- **Both rollups run on `not(canceled())` / `always()`**, not on
success. A check that reports **skipped** is treated by GitHub as
**satisfied**, so a required check that skips when its dependency fails
would wave a red build straight through.
- **`get-matrix.ps1` used to emit an empty matrix on a discovery miss.**
The downstream job is then skipped, a skipped job rolls up as a
*succeeded* stage, and the gate would have approved a run that executed
no tests. It now fails, with an `-AllowEmpty` opt-out.
- **Zero coverage reports used to be a green advisory check**, which
would hide a broken collector indefinitely. Both systems now distinguish
"the tests never ran" (warning) from "the tests ran and published
nothing" (error).
- The gate reads stage-level results rather than enumerating
matrix-generated job names, which change whenever a test project or
agent is added.

## Test-host hang and the reconnect defect behind it — both fixed

Flipping `CI_BUILD_BACKEND` to `actions` started exercising the ubuntu
matrix, which
`master` never runs, and it immediately surfaced two real defects.

**1. The test host hung for ten minutes.** `Assert.ThrowsAsync` blocks
the calling thread
with no timeout, and these tests drive a *fake clock* from that same
thread — so once the
reconnect parked on a fake-clock timer, nothing could ever advance it.
NUnit sat in
`WaitForCompletion` until `--blame-hang-timeout` killed the host,
aborting the run at 3902
of 4238 tests and taking every other test in the assembly down with it.
A hang dump named
the culprit exactly. Replaced with an awaited, bounded
`AssertThrowsAsync` helper (the
repo's own rules forbid sync over async). The run now completes in
**~1m29s** with all
tests executed.

**2. A budget-aware reconnect never returned.** With the hang gone, two
tests failed with
`Expected ServiceResultException but got TimeoutException`. `dumpasync`
on a hang dump gave
the whole chain:

```
ReconnectAsyncWithBudgetShrinksDelayToFitRemainingAsync
  → AssertThrowsAsync
  → ClientChannelManager.ReconnectAsync
  → ClientChannelManager.ReconnectLeaseAsync
  → ClientChannelManager.SwapFaultedEntryAsync   ← parked here
```

`master` (via #4128) added `IsTerminalReconnectRace` so a reconnect that
loses a race
against a concurrent close recovers on a freshly swapped entry. The
predicate only asks
whether the entry ended up `Closed`/`Faulted` with
`BadSecureChannelClosed` — which is
*also* exactly how a reconnect ends when the caller's retry budget or
the reconnect policy
is deliberately exhausted. So a budget-exhausted reconnect was misread
as a race: the
manager swapped the entry and started a **second, unbudgeted** cycle,
waiting out the swap
back-off first. `ReconnectAsync(channel, budget)` therefore could not
return within its
budget, and under a fake clock it never returned at all.

The fix requires the budget to have room left before treating a terminal
failure as a race:

```csharp
return !ct.IsCancellationRequested &&
    !budget.IsExhausted &&
    sre.StatusCode == StatusCodes.BadSecureChannelClosed &&
    entry.State is ChannelState.Closed or ChannelState.Faulted;
```

The parameterless `ReconnectAsync(channel, ct)` overload passes an
unlimited budget, so
genuine race recovery is unchanged. Verified locally: Core 4152, Client
2122 and Sessions
776 tests all pass, and the full GitHub Actions PR run is green across
56 jobs.

Two smaller fixes came along the way, neither weakening an assertion:

- `ObservableFakeTimeProvider` waiters made **relative** ("N more timers
from now") rather
than absolute ("the Nth timer of the run"). Absolute numbering let an
earlier reconnect
consume the slots, so a waiter completed before the awaited timer
existed and `Advance()`
  fired nothing.
- The raw worker thread in `NodeStateHandlerConcurrencyTests` was a
**foreground** thread
whose stop flag was read without a barrier and set only on the success
path — a failed
assertion leaked a spinning thread that pins the process at exit. Now
background,
  `Volatile`, stopped in a `finally`, joined with a bound.
## Background work now has an owner

The reconnect hang above was one symptom of a pattern the codebase had
**26 times** across seven assemblies: a bare `_ = Task.Run(...)`. Each
one handed work to the thread pool and forgot it — nothing observed the
exception if it threw, nothing bounded how many ran at once, and
disposal raced them, so a component could finish tearing itself down
while work it started was still touching its fields. Two of those sites
already carried a standing `// TODO: Await the task completion in
shutdown`.

`BackgroundTaskScope` (in `Opc.Ua.Types`) gives them an owner: schedule
with `Run`, drain with `DisposeAsync`, or signal-only with `Dispose`
where the owner's teardown is synchronous and awaiting would be sync
over async. `Run` never blocks and never throws — which matters, because
most of these call sites went to the thread pool precisely to escape a
lock they were holding. 11 unit tests cover drain, cancellation,
exception observation, the concurrency cap, post-shutdown rejection and
idempotent disposal.

Two of the fixes were more than re-wiring:

- **`SessionManager` / `SubscriptionManager` loops were structurally
unawaitable.** Both return `ValueTask`, so `Task.Factory.StartNew(() =>
...)` handed back a `Task<ValueTask>` that completes at the first yield.
Storing it would not have helped — awaiting it would only have awaited
the *scheduling*. Both now go through `AsTask().Unwrap()`, matching the
neighbouring `StartConditionRefreshWorker`.
- **`ChannelAsyncOperation` was swallowing subscriber exceptions.** Its
`doNotBlock` branch had no try/catch while the inline branch beside it
did. It is per-operation with no owner to hang a drain on, so it gained
the missing handler rather than a scope.

The Kubernetes readiness endpoint keeps a purpose-built semaphore
instead: an unauthenticated endpoint needs a hard cap on concurrent
handlers, not just an owner.

### ⚠️ Breaking: `ISessionManager.Shutdown()` is removed

Replaced by `ShutdownAsync(CancellationToken)`. There is no correct
synchronous way to wait for the session monitor loop, so leaving the
overload would only have preserved the race. This mirrors
`ISubscriptionManager`, which already had `ShutdownAsync`. Migration
notes are in `docs/MigrationGuide.md`; only one in-repo implementer
existed.

Also converted the injected `object partitionLock` shared by
`LogicalSubscription` and `CompositeMonitoredItemCollection` to
`System.Threading.Lock`, per the repo rule.

## Patch coverage now scales with the size of the patch

A flat floor over changed lines punishes small changes for arithmetic:
two uncovered lines in a four-line fix reads as 50 % and failed an 80/5
floor — which is exactly how this branch earned a red coverage check for
a four-line predicate change. A check that fires on changes nobody
considers undertested is one people learn to ignore.

| Coverable changed lines | Floor | Below it |
| --- | --- | --- |
| 1 – 10 | 50 % | warning, still passes |
| 11 – 100 | 60 % | warning, still passes |
| more than 100 | 75 % | **failure** |

Only changes past the last band can fail. Below it the author still gets
a warning naming the uncovered lines. Bands live in `patch.bands` in
`coverage-thresholds.json` and each carries its own `enforced` flag.
Verified end-to-end against synthetic repositories at every boundary.

## Codecov is back, as reporting

Dropped in #4122 when the in-pipeline gate replaced it. Keeping the gate
was right, but dropping Codecov also lost the PR comment, the
file-by-file diff view and the trend. It returns with both statuses
`informational: true` — two gates with two sets of thresholds would
eventually disagree, and `coverage-thresholds.json` stays the single
source of truth.

The upload is switchable and can never fail a build: `enableCodecov`
(Azure) and `ENABLE_CODECOV` (Actions), both defaulting on, both skipped
when `CODECOV_TOKEN` is absent as on fork PRs. Each system uploads the
single merged report it already builds, under its own flag. The old
`codecov.io/bash` uploader was sunset, so Azure uses the current CLI.

## Follow-up actions for maintainers

1. **Update the branch ruleset.** The per-leg `Coverage Release (…)`
checks are gone. Azure reports checks as `<pipeline> (<stage> <job>)`,
so the exact names are:

   | Check | Require? |
   | --- | --- |
| `OPCFoundation.UA-.NETStandard (Tests passed Verify stage results)` |
**yes** |
   | `build-and-test summary` | **yes** (already required) |
| `OPCFoundation.UA-.NETStandard (Code coverage Merge and evaluate)` |
no |
   | `code coverage` | no |
2. **Re-calibrate the thresholds.** `coverage-thresholds.json` is still
seeded from the last Codecov reading (73.61 %). The two systems now
report *different* numbers by design — Actions merges every project on
ubuntu, Azure only the Windows fast-PR legs, and scheduled runs read
higher still because the Debug / .NET 8 / .NET 9 / netstandard stages
also contribute. Re-seed from the GitHub Actions figure.
3. **Note:** a failed advisory coverage stage still turns the whole
Azure run red, which the README build badge reflects on `master`. That
follows from wanting a visible red X on a threshold miss; say the word
if you would rather it were a warning.
4. **The Azure agent pool is the slowest thing here.** Several runs on
this PR sat queued for hours, and one build failed with `Insufficient
system resources` while writing an XML doc file — an agent exhaustion
error, not a code error. It passed on re-run. Worth raising against the
`netstandard` Managed DevOps Pool capacity separately.
## Related Issues

_No tracking issue; this is CI infrastructure work requested directly.
It builds on the switch introduced in #4122._

## 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.
- No product code changed, so there are no unit tests to add. The new
logic was verified with purpose-built harnesses instead:
- `check-coverage.ps1` run for real against a merged Cobertura report on
all three paths — pass, threshold failure, and patch coverage with
uncovered changed lines (a scratch commit adding an uncovered method was
correctly resolved to `DataTypeException.cs:193`, reported 0 % patch
coverage and exited 1).
- The gate's accept/reject logic dry-run over `Succeeded` /
`SucceededWithIssues` / `Skipped` / empty / `Failed` / `Canceled` — 6/6
correct.
- `get-matrix.ps1` exit codes verified in clean processes: empty → 1,
normal → 0, `-AllowEmpty` → 0.
- All five pipeline/workflow files parse; both PowerShell scripts parse.
- Action versions checked against the GitHub API and pinned to current
majors.
- [x] I have added all necessary documentation.
- Rewrote the coverage section of `docs/DeveloperGuide.md` as *Required
checks and coverage*, covering which checks belong in the ruleset and
which must not, how coverage is measured, where the numbers appear, and
why the two systems disagree. Corrected the stale "BLOCKS" wording in
`coverage-thresholds.json`.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- CI YAML, two PowerShell scripts and documentation only; no compilation
involved.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- Not applicable — no product code changed. `Opc.Ua.Core.Schema.Tests`
was run repeatedly on net10.0 (119 passed) to generate the real
Cobertura fragments used to validate the gate.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [x] I have addressed **all** PR feedback received.
- Feedback on the path-relevance diff (now a merge-base diff) and the
target-framework numbering in `docs/DeveloperGuide.md` is fixed in
`15b8e38df`.
- A review pass over the gate/coverage split produced the four
correctness fixes listed above, in `863eefc4d`.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 26aa55a2-5731-4a4c-b61c-df2f7a785eca
 2026-08-07 06:56:05 +02:00
Marc SchierandCopilot e73e71184d [WotCon] Add OPC UA WoT Connectivity 1.1 with the Types, source generation, Server and xRegistry work it builds on (#4128) 
## Summary

Adds OPC UA WoT Connectivity 1.1 to the stack, together with the Types,
source generation, Server and xRegistry work it is built on. This branch
is the merge point for the fourteen-PR stack that replaced integration
PR #4093; every constituent PR was reviewed and approved separately and
is listed below.

414 files changed, +108,936 / -3,199. Roughly 46k added lines of product
code, 57k of tests, plus samples, tools and documentation.

## Types

- Complete `ExtensionObject` raw-body decoding for binary, XML and JSON
bodies by resolving the concrete type through the message context's
encodeable factory, and simplify the `Variant` structure helpers by
delegating to it (#4128).
- Add `IAtomicFileReplace` as an optional `IFileSystem` capability so
existing external implementations keep working, with atomic publish for
`LocalFileSystem` via `File.Replace`/`File.Move` and for
`VirtualFileSystem` by re-keying the in-memory entry (#4128).
- Add lossless conversion between WoT documents and NodeSet2
(`Opc.Ua.Wot.WotNodeSetConverter`). A byte-exact `uav:nodeSet` envelope
is preserved when requested, the structured `uav:nodes` projection is
used when the readable vocabulary is incomplete, and NodeSet2 is
otherwise synthesized from readable WoT terms. Unmapped JSON members
survive a round trip as pointer-addressed residue (#4131).

## Source generation

- Improve the NodeSet to ModelDesign conversion and the generated node
state (#4132).
- Generate OPC UA models directly from WoT Thing Description files
(#4134).

## Server

- Add live NodeManager shadow and immediate reload. A reload
materializes a new generation beside the active one and switches
atomically; the superseded generation is retired gracefully so existing
MonitoredItems keep being served until they drain, or immediately when
the caller asks for it. Includes the request admission and drain
machinery that lets an orderly shutdown wait for admitted requests
instead of tearing down underneath them (#4147).
- Make subscription transfer between sessions transactional, so a failed
transfer leaves neither session holding a partially moved subscription
(#4150).

## xRegistry

- Allow clients to use explicit registry roots rather than assuming a
well-known location (#4130).

## WoT Connectivity

- Add the WoT Connectivity 1.1 information model: a registry-first
revision layered on the abstract xRegistry base model, incorporating the
complete published OPC 10100-1 v1.02 surface as deprecated nodes in the
same namespace so existing 1.02 clients keep working (#4139).
- Add the protocol binding abstractions and planners that compile WoT
forms into executable plans (#4140).
- Add the HTTP, Modbus, OPC UA and MQTT binding executors (#4144).
- Add the registry and its stores, including document versioning,
validation, dependency resolution and refresh (#4146).
- Add the registry client (#4142).
- Materialize WoT Thing Descriptions into the server address space,
deriving types from Thing Models and instances from Thing Descriptions
(#4154).
- Restore the WoT asset registry bridge so the deprecated 1.02 asset
surface is backed by the registry (#4171).
- Add the WoT Connectivity samples and complete the documentation
(#4156), and restore the remaining coverage and docs (#4172).

## New projects

`src/Opc.Ua.WotCon.Bindings`, `src/Opc.Ua.WotCon.Bindings.Mqtt`,
`tests/Opc.Ua.WotCon.Bindings.Tests`,
`tests/Opc.Ua.WotCon.Samples.Tests`, and the `samples/WotCon`
AggregationServer, AggregationClient and FlatTagServer samples.

## Constituent pull requests

Merged into this branch top-down, each independently approved:

| PR | Title |
| --- | --- |
| #4128 | [Types] Complete ExtensionObject decoding and add an atomic
file replace capability |
| #4131 | [Types] Add lossless conversion between WoT documents and
NodeSet2 |
| #4132 | [SourceGeneration] Improve NodeSet to ModelDesign conversion
and generated node state |
| #4134 | [SourceGeneration] Generate OPC UA models from WoT Thing
Description files |
| #4147 | [Server] Add live NodeManager shadow and immediate reload |
| #4150 | [Server] Make subscription transfer between sessions
transactional |
| #4130 | [XRegistry] Allow clients to use explicit registry roots |
| #4139 | [WotCon] Add the WoT Connectivity 1.1 information model |
| #4140 | [WotCon] Add the WoT protocol binding abstractions and
planners |
| #4144 | [WotCon] Add the HTTP, Modbus, OPC UA and MQTT binding
executors |
| #4146 | [WotCon] Add the WoT Connectivity registry and its stores |
| #4142 | [WotCon] Add the WoT Connectivity registry client |
| #4154 | [WotCon] Materialize WoT Thing Descriptions into the server
address space |
| #4156 | [WotCon] Add the WoT Connectivity samples and complete the
documentation |
| #4171 | Restore WoT asset registry bridge |
| #4172 | Restore WoT coverage and docs |

Supersedes #4093.

## Follow-up

The WoT Connectivity and WoT Binding drafts have moved to 1.1-draft2
since this work was authored. Aligning with that revision - the removed
group vocabulary, the new projection/View construct, the remaining model
vocabulary terms and the two new portable-identity validation rules - is
tracked separately and will follow in its own pull request.

## Validation

Built for every target framework with zero warnings and zero errors, and
validated at the tip of the stack with `Opc.Ua.WotCon.Tests` at 999
passed / 0 failed and `Opc.Ua.Server.Tests` at 4024 passed / 0 failed.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
 2026-08-05 10:14:15 +02:00
CopilotmarcschierJulius Pfrommer
 		5a690c5a7c Address WSS discovery review feedback and follow-up tests (#4178) 
# Description

Addresses the review feedback for application-aware WSS discovery and
incorporates the follow-up coverage from #4179.

- Observe the returned task when asserting null
`ApplicationConfiguration` validation with `Assert.ThrowsAsync`.
- Correct the discovery URL comment grammar.
- Verify WSS discovery uses the application's certificate validator by
rejecting an untrusted discovery certificate and asserting the TLS
authentication failure.
- Connect the anonymous WSS binary test through the discovered
`ConfiguredEndpoint`, then verify the selected transport profile and
security mode.

## Related Issues

- Supersedes #4179.

## 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.
- [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: Julius Pfrommer <julius.pfrommer@o6-automation.com>
 2026-08-04 21:17:08 +02:00
Marc Schier 00664404c0 Ship the OPC Foundation license file from every package and fix the NU5033 pack failure (#4174) 
# Description

The `preview-pack` stage of the Azure DevOps pipeline fails with nine `NU5033` errors, which fails the whole build:

```
NuGet.Build.Tasks.Pack.targets(222,5): error NU5033: Invalid metadata.
Cannot specify both a PackageLicenseExpression and a PackageLicenseFile.
[src\Opc.Ua.Robotics\Opc.Ua.Robotics.csproj]
```

`common.props` gives every package the shared `LICENSE.txt` as its `<PackageLicenseFile>`, but **19** packages also declared `<PackageLicenseExpression>MIT</PackageLicenseExpression>`. Nine of them hit `NU5033`; the other ten only avoided it by carrying an empty `<PackageLicenseFile></PackageLicenseFile>` workaround.

That declaration was wrong on two counts.

**It advertised the wrong license.** `LICENSE.txt` is the **OPC Foundation MIT License 1.00**, which carries the OPC Foundation copyright and preamble — it is a different text than the plain SPDX `MIT` license. The affected packages therefore published `<license type="expression">MIT</license>` plus a `licenses.nuget.org/MIT` URL on nuget.org, instead of the OPC Foundation text that every other package in the stack ships.

**It also broke the build,** because NuGet refuses to pack a project carrying both a license expression and a license file, failing `pack` solution-wide rather than just for the offending projects.

## Changes

The expression is dropped from all 19 packages so every one uses the shared OPC Foundation license file uniformly, like the rest of the stack:

* `src/Opc.Ua.OpenUsd`, `.Client`, `.Server`
* `src/Opc.Ua.OpenUsdScene`, `.Server`
* `src/Opc.Ua.Robotics`, `.Client`, `.Server`
* `src/Opc.Ua.Positioning`, `.Client`, `.Server`
* `src/Opc.Ua.Di`, `src/Opc.Ua.ISA95`, `src/Opc.Ua.WotCon`, `src/Opc.Ua.Gds.Common`
* `tools/Opc.Ua.OpenUsd.Connector.Viewer`, `tools/Opc.Ua.SourceGeneration.Core`
* `tools/Opc.Ua.SourceGeneration.Pack`, `tools/Opc.Ua.SourceGeneration.Stack.Pack` — these two set the license not in their own `.csproj` but in the shared `tools/SourceGeneratorPack.targets`, so they were only caught once the new guard ran in CI.

The eight per-project empty `<PackageLicenseFile></PackageLicenseFile>` workarounds go away with it.

To stop the whole class of failure from recurring, `Directory.Build.targets` now *enforces* the rule rather than papering over it. Silently clearing the inherited license file would have fixed the build while still publishing the wrong license metadata, so instead a violation fails fast with an actionable error naming the project:

```
error OPCUA0001: Opc.Ua.Foo sets PackageLicenseExpression=MIT. Every package must
ship the OPC Foundation MIT License 1.00 via the shared LICENSE.txt that
common.props sets as PackageLicenseFile, which the SPDX expression neither
matches nor is allowed to coexist with (NU5033). Remove the
PackageLicenseExpression property.
```

`common.props` cannot catch this itself: it is imported from `Directory.Build.props`, i.e. **before** the project body, so it never observes a project-level license expression. `Directory.Build.targets` is imported **after** the project body and is the earliest place that can. The `common.props` comment that previously hinted the expression was an option now points at this rule.

### Verification

* Every packable project evaluates to `PackageLicenseFile=LICENSE.txt` with an empty `PackageLicenseExpression`; file-licensed packages such as `Opc.Ua.Client` are unchanged.
* `dotnet pack -c Release` succeeds for `Opc.Ua.Robotics` (previously failing) and `Opc.Ua.SourceGeneration.Core` (previously relying on the removed workaround).
* Replicated the CI build commands locally — `dotnet build` of `UA.slnx` (net48/Debug), `tools/SourceGeneration.slnx` (net472/Release) and `tools/MigrationAnalyzer.slnx` (net48/Debug) all succeed with 0 errors.
* Inspected the produced `.nupkg`: now `<license type="file">LICENSE.txt</license>` with the license text at the package root, matching the rest of the stack (it was `<license type="expression">MIT</license>` before).
* Confirmed the guard fires on a simulated violation (`-p:PackageLicenseExpression=MIT`) and is inert otherwise.

## Related Issues

No tracking issue — this fixes the `NU5033` failure in the OPC Foundation Azure DevOps pipeline (build 16247, `preview-pack` stage) and corrects the license metadata published for the affected packages. No product code or public API changes.

## 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. _(Build-infrastructure change with no code surface. Validated by `dotnet pack` of the affected projects, by inspecting the resulting package metadata, and by confirming the new `OPCUA0001` guard fires on a simulated violation. The pipeline `preview-pack` stage is the regression test.)_
- [x] I have added all necessary documentation. _(The rule is documented in place, on the new `Directory.Build.targets` guard and at the `PackageLicenseFile` definition in `common.props`.)_
- [x] I have verified that my changes do not introduce (new) build or analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
 2026-08-04 18:49:50 +00:00
Julius PfrommerandJulius Pfrommer 74e272ee8d Use application certificate validation during WSS discovery (#4136) 
ConfiguredEndpoint refreshed endpoint descriptions with a
DiscoveryClient created only from EndpointConfiguration. That path did
not receive the application certificate manager, so WSS discovery could
not validate a server certificate trusted by the application before
opening a session.

Add an ApplicationConfiguration-aware endpoint refresh overload and use
it from the default and channel-manager session paths. Preserve the
existing overloads for compatibility.

The application-aware DiscoveryClient now supplies the configured
certificate validation for secure discovery transports while TCP and
legacy callers retain their existing behavior.

Specification:
https://reference.opcfoundation.org/Core/Part6/v105/docs/7.5.3

Tests:
UpdateFromServerWithNullApplicationConfigurationThrowsArgumentNullException
(1 passed, net10.0)

Tests: UpdateBeforeConnectUsesApplicationCertificateValidationAsync (1
passed, net10.0)

# Description

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

## Related Issues

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

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

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

## Checklist

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

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [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: Julius Pfrommer <julius.pfrommer@o6-automation.com>
 2026-08-04 10:54:04 +02:00
Julius PfrommerandJulius Pfrommer da3358d371 Jpfr/fix wss session discovery (#4135) 
# Description

WSS binary endpoints use the `opcua+uacp` sub-protocol and carry the
normal OPC UA Connection Protocol, SecureChannel, and Session layers.
User authentication should therefore behave like `opc.tcp`,
independently of whether mutual TLS is enabled for the WebSocket
connection.

`HttpsServiceHost` previously shared an HTTPS-specific rule with WSS:
when `HttpsMutualTls` was disabled, it removed the `Anonymous`
user-token policy from the endpoint description. This prevented an
anonymous OPC UA session even when anonymous access was explicitly
configured by the server.

This PR:

- Preserves the server’s configured OPC UA user-token policies for WSS
UA-SC UA-Binary endpoints.
- Retains the existing anonymous-token filtering for HTTPS transports.
- Adds an integration test that disables mutual TLS, verifies that the
WSS endpoint advertises `Anonymous`, and opens a complete anonymous OPC
UA session.
- Passes `ApplicationConfiguration` into endpoint discovery performed by
`updateBeforeConnect`, allowing WSS discovery to use the application’s
certificate manager.
- Applies certificate-aware discovery to both the default and
channel-manager session paths while preserving the existing
endpoint-refresh APIs.

## Verification

- Anonymous WSS binary session with `HttpsMutualTls = false`: passed.
- WSS `updateBeforeConnect` using application certificate validation:
passed.
- Null application-configuration validation: passed.
- Affected projects build without new warnings.

## Related Issues

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

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

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

## Checklist

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

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [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: Julius Pfrommer <julius.pfrommer@o6-automation.com>
 2026-08-04 10:53:40 +02:00
Marc SchierandCopilot a6a5275006 Add a PumpX-2000 datasheet and diagrams to the pump sample (#4138) 
# Description

The `PumpDeviceIntegrationServer` sample simulates OPC 40223 pumps but
had no product documentation, a thin nameplate, and a duty point that
was not physically self-consistent. This PR gives the sample an
**official-style product datasheet** for its simulated asset and aligns
the server to it, so the document and the running address space cannot
drift apart.

### New: `samples/PumpDeviceIntegrationServer/DATASHEET.md`

A vendor-style datasheet for the fictitious *SimPump Corp PumpX-2000*
(clearly marked as a simulated device):

- Nameplate table mapped to OPC UA browse paths **and namespaces** (DI /
Machinery / Pumps)
- Best-efficiency duty point, characteristic curves and a performance
table
- Operating limits, motor / materials / dimensions data
- Instrument list (tag → browse path → unit → `EURange`) and a Mermaid
process schematic
- Alarm trip points and the deterministic simulation profile

### Server aligned to the datasheet

- `PumpDatasheet.cs` — the datasheet expressed as compile-time
constants.
- `MaterialiseNameplate` — materialises the 19 optional identification
properties through the generator-emitted `AddXxx(context)` helpers, so
each carries the browse name, namespace and DataType declared by the
model (21 nameplate fields in total per pump).
- `WithIdentification` now assigns the full datasheet nameplate to
**every** simulated unit. Product-level fields are shared; unit-level
fields (`SerialNumber`, `AssetId`, `ComponentName`, `Location`,
`FabricationNumber`) are derived from the pump number, so `--pumps N`
yields a consistent nameplate for each instance.
- **Simulation rebuilt around the datasheet curves.** Volumetric flow is
the only independent variable; differential pressure, mass flow,
efficiency and shaft power are derived from `H(Q) = 32 − 0.0104·Q²`,
`η(Q) = 72·(1 − 0.6·((Q−25)/25)²)` and `P = ρ·g·Q·H/η`, so `P = Δp·Q/η`
holds at every tick. Previously every measurement was an independent
sine and the sample published 5 kW of shaft power for 0.05 kg/s at 2 bar
Δp.
- `EURange` values now come from the datasheet; `OverTempAlarm` reports
the `BearingTemperature` chain (the correct `SourceNode` for an alarm
whose limits are in Kelvin) rather than a boolean; `Cavitation` and
`MotorOverheat` are derived from suction level and bearing temperature
with hysteresis instead of `tick % n`.

### Diagrams

The sample README gains a simulated-device summary, a link to the
datasheet, and three Mermaid diagrams: **address space**, **startup /
hosting sequence**, and **simulation + alarm dataflow**.

### Drift protection

`PumpDatasheetConformanceTests` pins the nameplate, engineering ranges,
trip points, published-value envelope and hydraulic consistency against
the datasheet, and the exact pump node-surface baseline in
`PumpInstanceNodeIdRegressionTests` covers the new identification
properties.

No changes to `src/` — this is sample, test and documentation only.

### Rebased on #4117

master has been merged into this branch. #4117 reshaped the same sample
(loop-based pump creation via `--pumps N`, `Pump_1` / `Pump_2`
BrowseNames with `Pump #n` DisplayNames, historian wiring,
`WireBoolean`, per-pump `Diagnostics` group). Both intents are
preserved:

- the datasheet nameplate is applied through upstream's per-pump
`WithIdentification(builder, pump, pumpNumber)` hook, so it scales to
`--pumps N`;
- `Program.cs` is left exactly as upstream has it — identification is
configured centrally, so the old per-pump block there is gone;
- the datasheet curve values are published through upstream's
status-code / source-timestamp plumbing, and upstream's `WireBoolean`
(TrueState/FalseState text + history) is kept;
- the node-surface baseline is the union of both identification lists;
- `docs/SourceGeneratedNodeManagers.md` was consolidated upstream into
`docs/NodeManagers.md`, so the snippet fixes were ported there.

## Related Issues

_No tracking issue; this is a documentation and sample-quality
improvement to the pump reference sample._

## 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 (post-merge)

- `dotnet build samples/PumpDeviceIntegrationServer` (net10.0): 0
warnings, 0 errors
- `dotnet build tests/Opc.Ua.Di.Tests` (net10.0): 0 warnings, 0 errors
- `dotnet test tests/Opc.Ua.Di.Tests -f net10.0`: **331/331 passed**
(includes upstream's new `PumpAddressSpaceComplianceTests` and
`DiConformanceFacetTests`)
- `dotnet test tests/Opc.Ua.Di.Tests -f net48 --filter Pump`: **44/44
passed**
- Live client read against the merged sample: Δp 214 079 Pa, ṁ 8.654
kg/s, η 69.33 %, P 2 677.4 W — `Δp·(ṁ/ρ)/η` matches the published shaft
power; `Pump_1` reads SN-001 / PMP-1001 / Feed Pump A / Bay 3 and
`Pump_2` reads SN-002 / PMP-1002 / Feed Pump B / Bay 4, exactly as
documented.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe608993-f3c9-4779-a6b9-9a9eabfc24ba
 2026-08-04 10:37:36 +02:00
Marc SchierandCopilot 81d726b7ad Coordinate the robot cell, and make the twin show what it is actually doing (#4162) 
# Description

`MinimalRobotServer` drove its two mobile manipulators through each
other. The platforms *did* clear — R1 and R2 trace figure-eights 1.2 m
apart at closest approach, exactly as `README.md` claimed — but each
**arm reaches 1.611 m** (link offsets `0.260 + 0.680 + 0.670 + 0.158`
from `robot.usda`), so the arms swept straight through one another while
the chassis politely missed. Clearance has to be argued about the
deployed arm, not the footprint.

Three related gaps sat alongside it: motion was open-loop (two
independent sine paths with no awareness of each other), only R1 had a
gripper, and nothing was ever actually picked — `PartA`/`PartB` were
static children of a single table and the arm replayed a canned
eight-pose loop unrelated to where either the robot or the table was.

This replaces all of that with a coordinated transfer cycle.

## Keeping them apart

The cell is divided into reservable zones, and a robot reserves the zone
*immediately ahead* before entering it, releasing the one behind once
clear — block signalling rather than a global schedule:

| Zone | Extent | Occupancy |
| --- | --- | --- |
| `EndZoneA` / `EndZoneB` | \|x\| > 1.3 m | one robot — a deployed arm
needs the whole end |
| `CorridorEastbound` | \|x\| ≤ 1.3 m, northern lane (y = 0.35) | one
robot |
| `CorridorWestbound` | \|x\| ≤ 1.3 m, southern lane (y = −1.15) | one
robot |
| `DockA` / `DockB` | around each charging dock (y = −2.0) | one robot |

Three rules follow:

- **Opposite-direction traffic passes**, being in lanes 1.5 m apart;
same-direction traffic queues rather than overtaking.
- **An arm may only leave its transport envelope inside an exclusively
reserved end zone.** This is the rule that actually fixes the reported
defect.
- **A robot with no work parks on its dock**, releasing the end zone so
the other robot can deliver into the station it serves.

That last rule is not cosmetic. Without it the cell deadlocks
permanently on the first handover: an idle robot standing beside its own
table blocks the very delivery it is waiting for. A state trace showed
R1 blocked at `x = 0.98` forever while R2 sat idle holding `EndZoneB`.
The docks are in a southern layby rather than beside the stations,
because a robot parked next to a station is close enough to foul one
turning into it.

## Where the part actually is

A carried part's pose is computed by forward kinematics
(`RobotKinematics`) from **the same six axis values the server
publishes**, not from the choreography that produced them — if the arm
and the part were computed independently they would visibly drift apart.
`RobotArmSolver` solves shoulder and elbow so the tool points down onto
a slot, and is checked against that same forward kinematics, landing the
tool on every slot of both tables to within 1e-6 m.

## Also simulated

Trapezoidal accel/cruise/decel travel with the heading following the
path; speed-and-separation slowdown near the other robot; an emergency
stop that now halts motion rather than only blinking the beacon; battery
drain with a charging detour; seeded grip-slip faults with floor
recovery; and KPIs (parts moved, cycle count, last/average cycle time,
utilisation, fault count). Both robots now carry a gripper.

## Two hidden couplings this surfaced

- `m_r1NodeId` was keyed on *"has a tool"*, so giving R2 a gripper
silently moved the dynamic-tool demo and the speed-override command
target onto R2. Now keyed on R1 explicitly.
- `RobotPositionsDriveLiveOpenUsdSceneAsync` sampled a fixed 1.2 s
window and assumed the robot was always moving. Under a duty cycle a
robot legitimately stands still while gripping, dwelling or charging, so
it now polls until movement is observed — the assertion's intent is that
live positions reach the scene, not that the robot never stops.

## Invariants are asserted, not assumed

`CellChoreographyTests` runs ten minutes of simulated time at 50 ms and
checks on **every step** that:

- oriented footprints never overlap (an oriented box, not a bounding
circle — the circle is so pessimistic for a 1.15 × 0.60 m platform that
it demands clearances no real cell would need);
- the two tool centre points never meet;
- a zone is never doubly occupied;
- arms only deploy inside an end zone;
- parts are conserved — never duplicated, lost or carried twice;
- a carried part sits exactly on the tool centre point.

Plus the round trip completing, KPIs accumulating, the emergency stop
halting every robot, dropped parts being recovered, and the battery
draining and recharging.

The README documents the zoning, traffic rules and cycle, **corrects the
misleading clearance claim**, and lists ten further realism extensions
that were deliberately not built (ISO 10218 / TS 15066
speed-and-separation monitoring, scanner cones, tool changer, inspection
station, conveyor infeed, gate interlock, torque telemetry, maintenance
counters, time-sampled pose history, traffic priority).

## Making the cycle visible

Running the sample end to end against the OpenUSD connector showed the
robots driving the cycle and the arms reaching the slots while **nothing
was picked up or moved**. The choreographer tracked part poses and
gripper opening purely in memory: the address space only ever exposed
the platform poses and the twelve axis angles, so a connector had
nothing to subscribe to for the parts and the blocks stayed wherever the
asset authored them.

A `CellTwin` folder now holds a `ThreeDCartesianCoordinates` position
and a `ThreeDOrientation` heading per part, plus a jaw position per
robot, bound to `/Cell/Parts/PartNN` and to the gripper jaws. The
variables are registered before the motion device system is built - a
binding can only carry a `SourceNodeId` that already exists - and sit
outside the robotics subtree, because a workpiece is not a device and
the robotics builder validates every node below its root as one it
allocated itself.

**The poses are published as structured vectors, not as a matrix.** A
viewer composes a prim's matrix from the vector translate and rotate
ops; the `matrix4d` profile is explicitly left unresolved by the
reference connector, which drops a binding it cannot convert *in
silence*. That is the shape the first attempt took, and it failed
without a word in any log.

The jaws needed the asset restructured. A carrier cube and a finger mesh
were siblings under the gripper body, so driving the carrier would have
slid it out from under the finger it is bolted to. Each jaw is now one
`Xform` owning both, declaring the single matrix op every live-bound
prim declares, with the finger points rebased onto the jaw origin.
Closed puts the finger faces on the 35 mm half-width of the block, so
the gripper grips it rather than passing through it.

`RobotAssetContractTests` now holds **every** live-bound prim to the
single-matrix contract, not just the two robot mount points.

## No part starves

Collecting the lowest free slot meant a part that never landed in slot 0
was never collected at all: with two parts seeded on `WorkTableA` the
second sat untouched for the life of the process while the robots
shuttled the other one past it - which reads in the twin as the robots
ignoring a block sitting in front of them. Parts are now stamped with
the order in which they came to rest and the one that has waited longest
is collected, which is what a real cell does with a buffer anyway.

Verified end to end: all three parts circulate between both tables, the
heading follows the carrying robot and resets on placement, and the jaws
stroke between open (0.060) and closed (0.047) as parts are gripped and
released.

The OpenUSD packages move to 0.3.0-alpha.

## The part was not in the gripper

Running it end to end showed a workpiece floating clear of the jaws that
were supposed to be holding it.

The platform pose and the workpiece pose leave the server as **separate
bindings published by different loops**: positioning at 5 Hz
(`UpdateIntervalMilliseconds = 200`), the simulation tick at 20 Hz. A
carried part rides the tool centre point, so it was being drawn against
a platform up to four ticks stale. Measured against the live scene with
the arm stowed — where the part-to-platform distance is fixed by
construction — the part sat anywhere between **0.65 m and 0.90 m** from
its platform instead of at a single distance. A quarter of a metre of
drift, three part widths, in front of a 96 mm jaw opening.

The platform now publishes at the tick rate, and a carried part is
derived from the pose that was **actually written to the address
space**, recorded at the `SetFrameValue` call site rather than where the
sample is produced. Each value was individually correct the whole time;
only their relationship was wrong, which is why nothing caught it.

`RobotOpenUsdE2eTests` now rebuilds the tool centre point from the
values authored into the scene and asserts a carried part sits on it. It
reproduced the defect at 0.49 m before the fix. The tolerance is the
sampling straddle of reading two independently delivered bindings — one
publish period is 43 mm at cruise — leaving an order of magnitude
between it and the defect.

Two assertions in the positioning test wanted the robots a metre apart
**in X**. They pass each other in opposite corridor lanes, where X is
all but identical and the separation is entirely in Y; and before the
first sample arrives both platforms sit on the zero frame and coincide
exactly. They now measure separation in the plane, once both have been
seen to move — this was failing intermittently on net48 already.

## The opening view

Framing the bounds of an enclosed scene automatically puts the eye
*inside* it: the viewport opened on a close-up of whichever robot
happened to be nearest, looking through the fence.

The cell now authors `/Cell/OverviewCamera` — the whole floor, both work
tables, the docks and the controller cabinet, from 12 m out at 28
degrees of elevation — and the connector opens on the first camera the
served root layer declares. It already had the plumbing to honour a
camera path; it was simply never given one. `--camera` still overrides
the choice outright, and `RobotAssetContractTests` asserts the
establishing shot is ordered before `/Cell/TopDownCamera`.

## The mounted gripper was never reported

`DynamicToolIsComposedAsync` was failing on both TFMs, in isolation, and
a longer timeout did not help. It predates this branch — I confirmed it
at `58fd5e8ca` with a clean worktree. It turned out not to be a flaky
test at all.

**Part 5 §9.32.2** lets only a node carrying a `NodeVersion` property
trigger a `ModelChangeEvent`, and `AsyncCustomNodeManager` drops entries
for nodes that lack one. Neither the mounted tool nor the robot it hangs
off had a `NodeVersion`, so **every addition was filtered out**. Removal
kept reporting, because a deleted node is no longer in the manager's
index and so takes the *"not mine, pass it through"* branch of the same
filter.

That asymmetry is what made it look like flakiness. A connector starting
while the tool was mounted composed it and could still deactivate it
later; one starting in the six-second gap was never told the tool
appeared and never composed it **at all, for the life of the process**.
Which happened depended only on the phase the fixture reached the test
in — near-constant, hence a consistent failure rather than an
intermittent one.

Measured directly before the fix: model-change events arrived only on
detach, exactly 18 s apart, and every resolve returned zero components:

```
14:00:12.738 SERVER mounted        <- no event
14:00:24.752 SERVER detached
14:00:24.801 model-change event    <- only this
```

After enabling model-change tracking on the robot, the prim tracks the
cycle — `A` = composed active (12 s mounted), `i` = inactive (6 s
detached):

```
....AAAAAAAAAAAAAAAAAAAAAAAiiiiiiiiiiiiAAAAAAAAAAAAAAAAAAAAAAAAiiiiiiiiiiii
```

**The test now asserts a transition**, not a momentary state: the prim
seen both active and inactive across more than one full cycle. Unlike
the old check this cannot pass on a connector that only ever hears about
one direction — which is exactly what the previous pair of tests could
not distinguish, since the sibling
`DynamicToolIsDeactivatedWhenDetachedAsync` was satisfied by the
*initial* compose and never exercised reconciliation at all.

Two robustness gaps in the connector fell out of this: a rejected
model-change monitored item was reported as success, leaving every
`Dynamic` component frozen at whatever it resolved at start-up; and the
initial compose ran *before* the subscription existed, so a change in
between was seen by neither. The first now fails loudly, the second
re-resolves once after subscribing.

`Opc.Ua.Di.Tests` is now **402/403 on net48** (was 3 failures) and on
net10.0, across three consecutive runs of the dynamic-tool tests on
each.

## Related Issues

- Fixes #4161

## 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: full `dotnet build UA.slnx -m:1
/p:UseSharedCompilation=false` — 0 errors, no new warnings.
`Opc.Ua.Di.Tests` 399/400 on **both net10.0 and net48** (1 pre-existing
skip), `Opc.Ua.OpenUsd.Tests` 635, `Opc.Ua.Robotics.Tests` 103.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
 2026-08-04 06:58:29 +00:00
Marc SchierCopilotCopilot Autofix powered by AI
 		724ae71f8a Make NodeId assignment during a node copy explicit (#4157) 
Fixes #4137.

## The problem

`NodeState.Initialize(ISystemContext, NodeState)` copies a node. Every
child it materialises through `CreateChild` gets a per-instance NodeId
from `ISystemContext.NodeIdFactory`, and is then initialised from its
source on the very next statement - which overwrites that NodeId. The
assignment is therefore *always* discarded: it only burns identifiers,
and permanently leaks them for factories that track outstanding
allocations. Booting the Robotics node manager left ten reservations
behind that every later allocation skipped.

#4123 fixed the symptom by hiding the factory for the duration of the
copy, wrapping the context in one that reports `NodeIdFactory` as
`null`. The review objection was fair: a context that lies about one
property is hard to reason about, and it only works because that
property is allowed to be null.

## What this does

States the intent instead of faking the environment. NodeId assignment
is now an argument of the two members that materialise children:

```csharp
public virtual BaseInstanceState? CreateChild(
    ISystemContext context,
    QualifiedName browseName,
    bool assignInstanceNodeIds = true);

protected virtual BaseInstanceState? FindChild(
    ISystemContext context,
    QualifiedName browseName,
    bool createOrReplace,
    BaseInstanceState? replacement,
    bool assignInstanceNodeIds = true);
```

A copy passes `assignInstanceNodeIds: false` and every node type -
generated or hand-written - sees the real context. There is no
capability property, no wrapper, and no second overload to keep in sync.

`MethodState`, `BaseDataVariableState` and the source generator template
each collapse to a single `FindChild` override that threads the argument
into its `CreateOrReplace<Child>` helpers.

## Breaking change

The four argument `FindChild` and the two argument `CreateChild`
virtuals are gone.

- **Call sites are unaffected.** The new parameter defaults to `true`,
so `CreateChild(context, browseName)` and `FindChild(context,
browseName, true, null)` keep compiling and keep the 1.5.378 behaviour.
- **Overrides must add the parameter.** A node manager that overrides
either member gets `CS0115` until it does. The compiler points at every
site; `docs/migrate/2.0.x/node-states.md` shows the before/after.

This is deliberate, per review: the stack ships all base types, and
types generated from a model are regenerated for 2.0 anyway.

## Also fixed

`MethodState.FindChild` returned `OutputArguments` when asked for
`InputArguments` with `createOrReplace: false` - a pre-existing bug in
the method being restructured. Covered by
`FindChildReturnsTheRequestedArgumentsProperty`.

## Validation

- Full `dotnet build UA.slnx`: **0 errors, 0 warnings**.
- `Opc.Ua.Types.Tests`, `Opc.Ua.SourceGeneration.Core.Tests`,
`Opc.Ua.SourceGeneration.Tests`, `Opc.Ua.Di.Tests`,
`Opc.Ua.Robotics.Tests` - all passing.
- Each new regression test was checked to fail without its corresponding
fix.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Copilot-Session: f45109d9-59cb-4ffd-a4f7-16e1ea449ae4
 2026-08-03 20:36:32 +00:00