2556 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Marc SchierandCopilot	df44b460be	Complete CreateInstanceOf node lifecycle during registration (#4360) ... # Description Generated `CreateInstanceOf<Type>` factories build typed node graphs and assign per-instance NodeIds, but intentionally leave the node create lifecycle open. Hand-written callers commonly registered those graphs without calling `CreateAsPredefinedNode`, so `OnBeforeCreate`, `OnAfterCreate`, and change-mask clearing never ran. This change: - adds `NodeState.IsCreated` and makes lifecycle completion idempotent per node; - completes newly materialized children, including children created by `OnAfterCreate`; - bounds late-child lifecycle convergence and fails explicitly if callbacks keep adding uncreated nodes; - completes uncreated nodes automatically before predefined-node registration and behavior wiring in both async and legacy custom node managers; - assigns/rebases async-node-manager NodeIds before lifecycle callbacks; - emits a null-safe Debug diagnostic when registration performs completion; - documents the generated-factory lifecycle contract and migration behavior; - adds regressions for condition/alarm method binding, finite-state-machine namespaces, Fluent state-node rebasing, behavior replacement, cloning, deletion, late children, non-converging callbacks, and synchronous/async registration. The tracked `src/` and `samples/` call-site audit covered 2,953 C# files. No additional ordering-sensitive call sites require explicit completion beyond the three existing DI/Robotics sites. ## Validation - `dotnet build UA.slnx -c Release -p:CustomTestTarget=net10.0`: passed. - `Opc.Ua.Server.Tests` net10.0: 4,811 passed, 5 skipped. - `NodeStateTests` net10.0 and net48: 213 passed on each. - Node-manager lifecycle and null-logger tests net48: 459 passed. - Source-generation core tests net10.0 and net48: 3,802 passed, 8 skipped on each. - DI, Robotics, ISA95, Positioning, Vision, Vision.Intent, and XRegistry suites passed on net10.0 and net48. - Generated alarm lifecycle regression passed on net10.0 and net48. The solution build reports six pre-existing CA warnings in untouched files. The OpenUSD suite has one pre-existing locale-sensitive failure in `StructuredCartesianCoordinatesAreAccepted` (`1.5` expected versus `1,5` under `en-DE`); all other OpenUSD tests passed on both tested frameworks. ## Related Issues - Fixes #4320 ## 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: 7ffdc400-0710-4a2e-a9d1-c0e1a9b3b81e	2026-09-02 13:08:49 +02:00
4 people	ede0b68154	Fix reverse-connect certificate leak and stale-connection retry (#4386) ... Two related robustness fixes on the reverse-connect / UA-SC handshake path, both surfaced by the Sessions suite. ## 1. Certificate handle leak in the asymmetric header parse `UaSCBinaryChannel.ReadAsymmetricMessageHeader` parses the sender certificate chain (`Utils.ParseCertificateChainBlob`, allocating fresh `Certificate` handles) and then runs several validations that can throw — invalid/missing sender thumbprint, no matching receiver certificate, receiver thumbprint mismatch, missing receiver thumbprint. The chain is returned through an `out` parameter, so a throw never reaches the caller's `using (senderCertificateChain)` in `ReadAsymmetricMessageAsync`, and the freshly parsed chain is abandoned. For a self-signed leaf that is exactly the intermittent `created == disposed + 1` seen on the Release leak gate under coverage instrumentation. This is the third sibling of the two concurrent-reconnect ownership leaks closed in #4356. **Fix:** wrap the post-allocation region in a `try/catch` that disposes the chain before rethrowing (validation logic unchanged). **Test:** `ReadAsymmetricMessageHeaderDisposesSenderChainWhenReceiverThumbprintMismatches` crafts a header with a valid sender certificate and a mismatched receiver thumbprint; without the fix it reproduces `created=3, disposed=2` and trips the assembly leak gate, with the fix it passes. ## 2. Stale reverse-connection retry A reverse connection the `ReverseConnectManager` delivers can already have been closed by the server before the secure-channel handshake runs: a server rotates and times out the outbound connections it offers, so the one matched here — especially after an `updateBeforeConnect` discovery round consumed an earlier one — may be stale. That surfaced as an intermittent `BadConnectionClosed` ("Remote side closed the connection") failing the whole connect (e.g. `ReverseConnect2Async(true,true,…)`). **Fix:** wrap reverse-connect session creation in `DefaultSessionFactory` in a bounded retry (3 attempts) that requests a freshly delivered connection when an attempt fails with a connection-drop status (`BadConnectionClosed` / `BadNotConnected` / `BadSecureChannelClosed`). Non-transient failures and cancellation still surface immediately; the bound prevents an unbounded loop against a genuinely unreachable server. ## 3. Test-host crash: NULL channel map in the inactivity timer `TcpTransportListener.DetectInactiveChannels` (a fire-and-forget inactivity-timer callback) iterated `m_channels` without holding `m_lock`, while `CloseAsync` disposes the timer and nulls `m_channels` under the lock. Disposing an `ITimer` does not join an already-dispatched callback, so a late callback dereferenced the now-null map and threw an unhandled `NullReferenceException` on a thread-pool thread, crashing the process. This intermittently aborted the `test-macOS-latest-Client` host. **Fix:** snapshot `m_channels` and return early when null (mirroring the existing guarded read on the accept path). Deterministic regression test invokes the callback on a listener whose channel map is null (its state before `OpenAsync` / after `CloseAsync`) and asserts it does not throw. ## Verification - `Opc.Ua.Core.Tests` Stack.Transport/Client/Server: 2178 passed, leak gate green. - Coverage-instrumented `Opc.Ua.Sessions.Tests` (the CI leg): 775 passed, 0 failed, leak gate green. - Reverse-connect suite (`ReverseConnect*`): 52 passed with the retry. - `Opc.Ua.Core` and `Opc.Ua.Client` build warning-free (net10.0, net48, netstandard2.1, net472). Note: the reverse-connect failure is a rare timing flake (0 reproductions in 88 local attempts), so fix #2 addresses the root cause identified from the ADO log and is validated by construction plus regression rather than by a captured local repro. ## Related - Follows #4356 (concurrent channel reconnect certificate leaks). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Marc <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-09-02 12:24:42 +02:00
Marc SchierandCopilot	b97fd8d4b5	Fix OPC 10000-21 onboarding interoperability (#4385) ... # Description Fix the four blockers that prevented the OPC 10000-21 onboarding registrar and client from working end to end: - generate every `ModelDesign` target independently, including multiple models in one directory, while preserving stable identifier CSV matching and cross-model dependencies; - ship the generated `Opc.Ua.Onboarding` constants, node states, encodeables, and typed clients from `Opc.Ua.Gds.Common`; - correct the standard `EncodedTicket` datatype to derive from `ByteString`; - align the new 2.0 `OnboardingClient` API with the generated `ArrayOf<ByteString>` / `ArrayOf<StatusCode>` contract; - make `BindToTicketStore` fully asynchronous, cancellation-aware, and assign the declared output slot instead of appending a second output; - add generated-model, client fallback, server binding, and in-process end-to-end regressions. The change also adds `samples/Gds/OnboardingRegistrar`, `samples/Gds/OnboardingClient`, and `run-onboarding-demo.ps1`. The demo loads the generated GDS and Onboarding models, restricts ticket administration to an ephemeral `RegistrarAdmin` user, proves anonymous access is denied, and completes a secure register/unregister round trip. The History CI follow-up rejects negative and non-finite processed-history intervals before provider dispatch and enforces the buffered aggregate output cap while calculator results are flushed. This prevents malformed requests from running until the client timeout, exhausting memory, delaying server teardown, and causing later alarm-event tests to fail. ## Related Issues - Fixes #4378 - Unblocks OPCFoundation/UA-.NETStandard-Samples#819 - Tracks the canonical model correction in OPCFoundation/UA-Nodeset#122 ## Validation - Full `net10.0` test projects: - `Opc.Ua.SourceGeneration.Core.Tests`: 3803 passed, 8 skipped - `Opc.Ua.SourceGeneration.Tests`: 143 passed - `Opc.Ua.Gds.Tests`: 1114 passed, 42 skipped - `Opc.Ua.History.Tests` with the CI coverage settings: 506 passed, 27 skipped - `HistorianDispatcher` tests: 88 passed on `net10.0`. - Invalid processed-history interval regressions passed on `net10.0` and `net48`. - Focused onboarding/source-generation regressions passed on `net48`. - `dotnet build UA.slnx -p:CustomTestTarget=net10.0 --no-restore` succeeded; the reported analyzer warnings are pre-existing in unchanged XRegistry/WoT files. - Packed all three GDS packages and verified the packaged `Opc.Ua.Gds.Common` assembly contains the generated Onboarding model surface. - `pwsh samples/Gds/run-onboarding-demo.ps1` verified anonymous denial and the authenticated secure ticket lifecycle. ## 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: 2d2c52c7-7040-4bd6-98de-aaf82ba7846b	2026-09-02 11:39:19 +02:00
3 people	029e4ce7f3	Fix ManagedSession.CloseAsync blocking during reconnect attempts (#4374) ... - [x] `ConnectionStateMachine.RequestClose()` cancels the connect/reconnect/failover attempt in flight (linked close token) instead of only flagging the state - [x] `WaitForClosedAsync` is released whenever the machine reaches `Closed`, including through dispose - [x] `ManagedSession.CloseAsync(timeout, closeChannel, ct)` honours `timeout` (bounded wait, `BadTimeout` on expiry) and `closeChannel`, and returns the real close status; `CloseAsync(ct)` now closes through the same path - [x] `Session.CloseAsync` publish drain waits only for the requests in flight at close time, skips when no response can arrive, and no longer issues `Cancel` round trips for already-completed requests - [x] Unit tests: `ConnectionStateMachineTests` (close cancels reconnect in flight, close during connect) and `ManagedSessionTests` (close completes while a connect attempt is in flight) - [x] `docs/Sessions.md` documents the close semantics - [x] Validation: Opc.Ua.Client builds for all TFMs; `Opc.Ua.Client.Tests` (2167 passed) and `Opc.Ua.Sessions.Tests` (775 passed) green on net10.0 - [ ] `Opc.Ua.Subscriptions.Tests` run was still in progress when time ran out <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #4364 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: romanett <romanett98@gmail.com>	2026-09-02 09:28:45 +00:00
romanettandClaude Fable 5	6e791517b5	Make CertificateStoreIdentifier a pure store description (#4382) ... ### Why Follows up on the [review discussion in #4357](https://github.com/OPCFoundation/UA-.NETStandard/pull/4357#discussion_r3903422013): the rule in this codebase is *if you get handed an IDisposable, it is your responsibility to dispose it*, but `CertificateStoreIdentifier.OpenStore` handed every caller the **one** store instance cached on the identifier. A caller honoring the rule disposed shared state other holders of the same identifier were still using (it only appeared to work because the store implementations tolerate use after dispose), while a caller that only called `Close()` left the cached instance — and its parsed-certificate cache — alive with no owner, which is exactly what the certificate leak gates flag at suite end. ### What Of the two options discussed (caller-owned instance vs. a new ref-counted wrapper type), this implements the simpler one, modeled on the existing `CertificateIdentifier` / `Certificate` pair: - `CertificateStoreIdentifier` is now a pure **description** of a store, exactly like `CertificateIdentifier` is for a certificate. It caches nothing and has no disposable-like lifecycle. - `OpenStore` creates and opens a **new instance on every call**, which the caller owns and must dispose — like a resolved `Certificate`. If `Open` throws, the instance is disposed before the exception propagates. - Components that access a store repeatedly keep their instance open for as long as they need it instead of relying on identifier-level caching: the certificate validator already does this via its per-core store cache (`CertificateValidationCore.m_stores`), which now also disposes the loser of its benign `GetOrAdd` race instead of leaking it. - Call sites that relied on the shared cached instance by only calling `Close()` now dispose the store they own: `CertificateTrustList.GetCertificatesAsync`, `SecurityConfiguration` store validation, both `X509Utils` store extensions, the server `TrustList` store-update helpers and the rejected-certificate list read. - New `CertificateStoreIdentifierOpenStoreTests` pin the ownership contract (independent instance per call, disposing one instance does not affect another, empty path returns null). A ref-counted wrapper prototype was implemented and discarded: it forced every short-lived identifier creation site (~20 in src, more in tests) to release a hidden base reference, spreading the disposable-like lifecycle the review objected to across the whole codebase instead of removing it. ### Verified Release, `net10.0`, with `OPCUA_CERTIFICATE_LEAK_TRACKING=1`: - Opc.Ua.Core.Tests: 4376 passed - Opc.Ua.Configuration.Tests: 227 passed - Opc.Ua.Server.Tests: 4797 passed - Opc.Ua.Gds.Tests: 1111 passed - Opc.Ua.Client.Tests (full suite): 2164 passed - Opc.Ua.Core.Security.Tests: 556 passed - Opc.Ua.PubSub.Mqtt.Tests: 157 passed (its strict certificate instance-count test passes unmodified under the new contract) - Opc.Ua.Tools.Tests: 710 passed 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-09-02 10:43:13 +02:00
romanettandClaude Opus 5	ff68296fda	Close the SupportsFilteredRetain gaps in the server implementation (#4377) ... Fixes #4370. `SupportsFilteredRetain` worked for the exact shape the unit tests exercised, but was unreachable for other filter targets, wrong for branches, and kept state that outlived the filter it was derived from. This addresses all seven gaps from the issue. ### 1 — reachable only via `InstanceStateSnapshot` `CanSendFilteredAlarm` now resolves the condition through a helper that accepts either a snapshot's `Handle` or a `ConditionState` passed directly. Anything else still degrades to plain `WhereClause.Evaluate`, which is now stated in the XML docs instead of left implicit. ### 2 — the commented-out `Retain` guard Behaviour is unchanged — that is a spec call — but the dead `if` is gone and the open question is written up in place with a link to the issue, so it is tracked somewhere more durable than a bare source comment. ### 3 — branches collide on the key The key is now composed of ConditionId **and** BranchId, so a condition and its branches no longer contend for one entry. ### 4 — branches never inherit the flag `CreateBranch` copies `SupportsFilteredRetain` into the branch explicitly. ### 5 — address-space wiring I checked the spec before touching this. Part 9 §5.5.2 says the Property *"is only provided on the ConditionType"*, and `i=32060` in the standard nodeset accordingly carries no `HasModellingRule` — so the generator and nodeset are right, and making it an instance child would be the deviation. Confirmed experimentally: flipping `StandardTypes.xml` to `ModellingRule="Optional"` does generate the full property / `GetChildren` / `FindChild` / `CopyTo` wiring, and its **only** address-space effect is stamping `ModellingRule_Optional` onto `i=32060` — which the standard nodeset does not have. That change is therefore not made here. The two halves are reconciled the other way instead: the C# property is documented as the server-side per-condition switch that is deliberately *not* an address-space child (clients read the flag from the type node, which the generated address space already builds), with a test locking that in. Since nothing walks it as a child, #4 is fixed by the explicit copy rather than by `Initialize`. ### 6 — `ConditionRefresh` bypassed the bookkeeping Fixed for free by 1. Refresh queues the raw `ConditionState`, which now resolves, so refreshed conditions are evaluated against the where clause and re-prime their entries. No new API was needed — the concern in the issue about refresh mutating the set turns out to be benign: an entry means "passed the where clause at the last evaluation", so a refresh of an in-scope condition removes and re-adds the same key. ### 7 — never cleared, never persisted `ModifyAttributes` discards the state when the where clause actually changes (select clauses are ignored — they only shape the fields of an event that is being sent anyway). Persistence added via `IStoredMonitoredItem.FilteredRetainConditionIds` → `StoredMonitoredItem` → `ToStorableMonitoredItem` / restore constructor, plus encode/decode in `SharedKeyValueSubscriptionStore` behind a new definition format version 3. ## Judgment calls worth a look - **`SetMonitoringMode` deliberately keeps the state.** A disabled item never reaches `CanSendFilteredAlarm` at all, so the entries still describe what the client was last told, and the trailing event is owed to it once reporting resumes. Documented rather than changed. - **`IStoredMonitoredItem` gained a member**, which is source-breaking for anyone with a custom subscription store. `TestStoredMonitoredItem` in the test suite needed the same one-line addition. ## Tests `tests/Opc.Ua.Server.Tests/FilterRetainTests.cs` grows from 16 to 26 tests, covering branches, `ConditionState` targets, filter modification (both changed and unchanged where clause) and a durable round trip. Each new test was verified to fail against the unfixed code by reverting the corresponding change individually. The suite also drops the reflection hazard the issue flagged: a `TestableMonitoredItem` subclass exposes the protected method, so a signature change now fails the build instead of the run. Its queue factory is owned by the fixture rather than disposed inside the call that hands it to the item. ## Verification Full solution builds clean. `Opc.Ua.Server.Tests` (4801), `Opc.Ua.Core.Tests` (4371), `Opc.Ua.Redundancy.Server.Tests` (573) and `Opc.Ua.History.Tests` (506) all pass on net10.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-02 07:02:43 +02:00
Marc SchierandCopilot	8ce8c30c21	Default null GetCertificates group to DefaultApplicationGroup (#4380) ... # Description Make `ServerConfiguration.GetCertificates` interpret `NodeId.Null` as the `DefaultApplicationGroup`, matching the other PushManagement certificate Methods. - Centralize certificate-group defaulting and validation in `ConfigurationNodeManager`. - Preserve `Bad_InvalidArgument` for unknown non-null group IDs and retain the existing authorization ordering. - Cover the behavior through direct server and public push-client tests. - Document the null-group behavior and correct the OPC 10000-12 reference to §7.10.8. ## Related Issues - Fixes #4379 ## Validation - `ConfigurationNodeManagerPushTests.GetCertificates*`: 5 passed on `net10.0` and 5 passed on `net48`. - `PushTest.GetCertificatesAsync` and `PushTest.VerifyNoUserAccessAsync`: 26 passed on `net10.0`; 10 passed and 16 unsupported ECC fixture variants skipped on `net48`. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4cd8e98c-7cbc-45bb-9ee8-2fdd7fd59e3c	2026-09-02 07:01:45 +02:00
Marc SchierandCopilot	81f200c9d2	Align WoT conversion and runtime with Binding 1.1 (#4375) ... ## Summary Align the generic WoT/NodeSet converter, WoT Connectivity registry, and runtime binding implementation with the OPC UA WoT Binding 1.1 draft. ### Conversion and preservation - fix the standard placeholder ModellingRule NodeIds; - make every produced/restored NodeSet importable by completing required aliases; - support `ua:ComponentOf`, arbitrary companion ReferenceTypes, BrowseNames, InverseNames, symmetric references, and exact direction; - enforce the projection annotation allowlist and deterministic total order; - emit and consume Method argument schemas, EventType data, Alarms & Conditions, event severity, engineering units/ranges, localization, ValueRank, and ArrayDimensions; - preserve unsupported content through JSON residue/native projection rather than silently dropping it; - compare residue conflicts with RFC 8785 canonical equality while retaining byte-exact residue digests; - stamp generated documents with `uav:bindingVersion: "1.1"`. ### Runtime bindings - resolve an EventType Thing Model referenced with `tm:ref`, recursively derive its `data` fields as the baseline selection, and overlay explicit `uav:eventSelectClauses` that also reference EventType TMs; - pre-resolve EventType links asynchronously through the bounded sibling-document resolver before synchronous protocol planning; - retain read compatibility with legacy implementation-specific `uav:eventFields`; - expose nested event `data` matching the TD while retaining the flat transport field index for compatibility; - apply minimum-security constraints through deterministic endpoint selection and fail-closed session verification; - use one Unicode code-point comparer for projection membership and endpoint ties. ### Validation and fixtures - add strict, profile-aware validation while keeping default parsing permissive and residue-preserving; - pin 27 normative examples byte-for-byte to the specification repository, with an offline SHA-256 manifest and regeneration test; - regenerate checked-in WoT documents with the final canonical writer and binding version; - document the generic converter separately from the legacy OPC 10100-1 asset path. ## Notable compatibility changes - converted NodeSets may gain a completed `<Aliases>` table so standard ReferenceType names can be imported; - previously inverted/dangling placeholder ModellingRules are corrected; - generated documents gain binding-version, localization, rank/dimension, and exact ReferenceType metadata; - event data now has the nested shape described by the binding; - the former NodeId-based `uav:typeDefinitionId` clause is removed; event selections now use EventType-TM `tm:ref` links; - `uav:eventFields` remains readable but new documents use `uav:eventSelectClauses`; - unknown well-formed future revision/profile claims produce unsupported warnings rather than strict rejection. ## Generated files Most of the 621-file diff is deterministic regeneration: 517 checked-in WoT documents gain the binding version and final canonical spelling. Regeneration was run twice and is byte-idempotent. ## Validation - `dotnet restore UA.slnx`; - `dotnet build UA.slnx -c Release` across all configured TFMs; - `Opc.Ua.Types.Tests`: 9,215 net10.0; 9,208 net48; 9,208 net472; - `Opc.Ua.WotCon.Bindings.Tests`: 611 each on net10.0, net9.0, and net8.0; - `Opc.Ua.WotCon.Tests`: 1,136 net10.0; - `Opc.Ua.WotCon.Samples.Tests`: 14 net10.0; - NativeAOT test executable: 136/136; - all 27 strict fixture conversion/import checks; - formatting, analyzers, `git diff --check`, and regeneration idempotence. The full net48 run retains one unrelated existing MAX_PATH failure in `FileWotRegistryStoreTests.MultiLevelDirectoryCreationSyncsEveryModifiedParentInOrder`. Solution-wide stress runs also include an intentional 60-minute soak and unrelated locale-sensitive/flaky tests; all affected WoT projects and tests pass. ## Specification Depends on OPCF-Members/spec-drafts#31. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a9f41b1-72f0-4c74-bd7f-26fa531e5738	2026-09-02 00:09:36 +00:00
4 people	76efb34ab8	Fix FileSystemClient path qualification and canonical paths (#4365) ... ## Summary - Qualify unprefixed `CreateDirectoryAsync` path segments with the current parent namespace. - Preserve qualified segments after cached and uncached resolution so returned `FullPath` values are canonical. - Keep explicit namespace-prefix validation tied to caller input and use the server-reported `BrowseName` after creating a directory. - Add unit coverage for both `createIntermediate` modes, canonical file and directory paths, and cached resolution. - Add an integration regression using the reference server, `FileSystemNodeManager`, and `PhysicalFileSystemProvider`. ## Testing - `Opc.Ua.Client.Tests` filesystem category on `net10.0`: 118 passed. - `FileSystemClientIntegrationTests` on `net10.0`: 14 passed. - `Opc.Ua.Client.Tests` filesystem category on `net48`: 118 passed. - `FileSystemClientIntegrationTests` on `net48`: 14 passed. Fixes #4362 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: Marc <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-09-01 20:30:15 +00:00
romanettandClaude Opus 5	70161bc145	Fix Part 18 RoleSet: AddRole overwriting nodes, and Roles configured at startup never getting one (#4367) ... # Description Two defects in the Part 18 Role management implementation added by #3735. They are separate root causes but both concern Roles that are not one of the nine well known ones. ## 1. `AddRole` could allocate a NodeId that is already in use `RoleManager.AllocateDynamicNodeId` counted up from `i=1` in whatever namespace it was given, with no check that the identifier was free. The namespace came from the `NamespaceUri` argument of the standard `RoleSet.AddRole` Method, so a client holding `SecurityAdmin` could pass the namespace URI of a node manager's own model and receive the first identifier in it — which every model compiler and the source generator have already used. `MaterializeDynamicRoleAsync` then created the Role at that NodeId, replacing the model's node in the address space of a running server. The root cause is that `NamespaceUri` was treated as selecting the namespace the Role's NodeId is allocated in. Per Part 18 §4.2.2 it qualifies the **BrowseName**; the NodeId is server-assigned. The fix works on three levels: - `RoleManager.AddRole` resolves `NamespaceUri` into the BrowseName namespace only — recorded as both an index and a URI on the new `RoleEntry.NamespaceUri`, so the binding can re-resolve it against the server namespace table — and always allocates the NodeId in `defaultNamespaceIndex`, which `RoleStateBinding` picks from a namespace the RoleSet's NodeManager owns. A client can no longer steer allocation into another node manager's model at all. This also fixes a second-order problem: a Role node materialized into a foreign namespace was indexed in the diagnostics manager but routed elsewhere by the `MasterNodeManager`, so it could never be resolved. - `AllocateDynamicNodeId` skips identifiers that are already taken — by another Role, or by a node in the address space, via a probe `RoleStateBinding` installs. This is the fix suggested in the issue. - Materialization refuses rather than replaces. An occupied NodeId answers `Bad_NodeIdExists`, one outside the manager's namespaces answers `Bad_NodeIdInvalid`, both logged; `AddRole` rolls the manager back so it is all-or-nothing and never leaves a Role that grants access but has no node. ## 2. A Role configured at startup was enforced but got no node in the `RoleSet` `RoleStateBinding.Initialize` discovered Roles by walking the address space only, and `MaterializeDynamicRoleAsync` ran from `OnAddRoleAsync` alone. `RoleConfigurationChangeKind.RoleAdded` for an unbound Role did nothing. Every documented way of configuring a custom Role before the server is up — overriding `StandardServer.CreateRoleManager`, `IOpcUaServerBuilder.ConfigureRoles(...)`, `AddRoleManager<TManager>()` with a pre-populated manager — therefore produced a Role that `MasterNodeManager.ValidateRolePermissions` honours but that a client cannot browse, read `Identities` from, or call `AddIdentity` on. Part 18 §4.2 expects the `RoleSet` to describe the Roles the server supports, so this is a conformance gap as well as an ergonomic one. - `InitializeAsync` now materializes every `IRoleManager.RoleIds` entry the address-space walk did not return. - `OnRoleConfigurationChanged` handles `RoleAdded` for Roles that are not bound yet, so a Role added by a custom `IRoleManager` at runtime also appears. The manager raises the event synchronously from inside itself, so that work is queued rather than awaited in the handler. - Both go through one idempotent, serialized `EnsureRoleMaterializedAsync`, which is also where the NodeId checks from defect 1 live. ## Third-order fix: `ConfigureRoles` allocated NodeIds against a throwaway namespace table `ApplyRoleConfiguration` ran inside the DI factory for `IRoleManager`, where the server's namespace table does not exist yet. It built a local `NamespaceTable` and defaulted to index 1, so a configured custom Role was allocated in a namespace the RoleSet's NodeManager does not own — which the refusal added above would then correctly reject, leaving the Role invisible again and defect 2 unfixed for the `ConfigureRoles` path. The options are now staged on the manager (`RoleManager.PendingConfiguration`) and applied by the binding once the address space is up, against the real namespace table. Settings on Roles that already exist — the nine well-known ones — are still applied eagerly in the DI factory, so resolving `IRoleManager` before start-up behaves as it did. ## Notes for reviewers - **`RoleStateBinding.Bind` becomes `BindAsync`.** Materializing needs `AddPredefinedNodeAsync`. The only caller was `ServerInternalData`, in an already-async method. This is a public API change on a type introduced in `2.0.0-preview.2`. - **`AddRole`'s returned NodeId namespace changes** for a call that passes a non-empty `NamespaceUri`: it is now the server-chosen namespace rather than the caller's. This is the fix, and the existing XML doc on `IRoleManager.AddRole` already described this behaviour ("a fresh NodeId is allocated in `defaultNamespaceIndex`") — the implementation was what deviated. The BrowseName namespace is unchanged. - **`AddRole` is now all-or-nothing.** Materialization failure used to be logged and swallowed, returning `Good`. It now rolls the manager back and returns the reason, per the issue's "refusing would at least turn silent corruption into a diagnosable error". - **`RoleEntry` gains `NamespaceUri`** and its internal constructor gains a parameter. - The unit fixture in `RoleStateBindingTests` now logs "not in a namespace owned by the RoleSet's NodeManager" for eight well-known Roles. That is a fixture artifact — its test node manager does not own `ns=0`, unlike a real `DiagnosticsNodeManager` — not a real-server condition. - Unrelated but adjacent: `AddRoleHandler_TypedRoleStateUpgradedByBinding_CanCallAddIdentity` added a Role named `Engineer`, which `AddRole` rejects as a duplicate browse name. Its `Assume` never passed and its body never ran, on master too. Renamed to a non-well-known name so it exercises what it claims. ## Testing - `RoleManagerTests`: NodeId allocated in the default namespace while the BrowseName keeps the requested URI, unregistered URI rejected, identifiers reported in use by the address space skipped, an existing Role's NodeId never reused. - `RoleStateBindingTests`: `AddRole` with a foreign model namespace leaves the model's own node at `i=1` untouched; a stub `IRoleManager` that hands back an occupied NodeId gets `Bad_NodeIdExists` and a rollback; a Role configured before the address space existed is materialized and referenced by the `RoleSet`; `ConfigureRoles` options are applied at bind time; a Role added on the manager after bind is materialized. - `ServerFluentApiHostingTests`: end-to-end on a hosted server — a custom Role from `ConfigureRoles` has a node under the `RoleSet` carrying its configured identity mapping. - `RoleManagementIntegrationTests`: end-to-end against the reference server over an encrypted channel — `AddRole` with the model namespace URI qualifies the BrowseName only and the Role is browsable. Locally on .NET 10: `Opc.Ua.Server.Tests` 4801 passed, `Opc.Ua.Features.Tests` 136 passed, `Opc.Ua.Core.Security.Tests` 556 passed, `UA.slnx` builds with no new warnings. ## Related Issues - Fixes #4361 Found while building an OPC UA Part 18 sample for OPCFoundation/UA-.NETStandard-Samples#818. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-01 19:28:43 +00:00
romanettandClaude Opus 5	3543d7cf6c	Report Executable/UserExecutable for state machine causes (#4368) ... # Description A Part 16 cause is a Method, and whether it may fire depends on the state the machine is in. Part 3 defines `Executable` / `UserExecutable` as exactly that question, and Part 4 has `Call` refuse a non-executable Method with `Bad_NotExecutable`. `FiniteStateMachineState` already knows the answer — `IsCausePermitted(context, causeId, checkUserAccessRights)` — but nothing outside alarm shelving connected the two. Across the library, `OnReadExecutable` was assigned in exactly three places, all in `AlarmConditionState`. Everywhere else a cause method kept the `true` its node was constructed with, so a client could not tell which causes currently apply and found out only by calling one and being refused with `Bad_NotSupported` from `DoCause`. ## What changed **`StateMachineBuilder.WithCause`** now answers both attributes from `IsCausePermitted` alongside installing the call handler — on the unified builder (via the dispatcher's `InstallCauseMethod`) and on the `Opc.Ua.Server.Fluent` surface reached through `INodeBuilder.AsStateMachine()`, which had the identical gap. `Executable` ignores user rights; `UserExecutable` also runs the machine's `OnCheckUserPermission` callback. Per the issue's note about servers that deliberately keep methods executable and answer at call time, both overloads take `reportExecutable: false` to opt out. **`ProgramStateMachineState`** wires its own `Start` / `Suspend` / `Resume` / `Halt` / `Reset` in `OnAfterCreate`, so a Program server no longer repeats the boilerplate the class already shipped handlers for. The Boiler quickstart's copy of it is deleted rather than left as an example. **Generator-emitted `*StateMachineState` subclasses** are left alone. The generator emits no Part 16 `StateTable` / `TransitionTable` / `CauseMappings` overrides — the same gap that makes the DI software-update dispatcher write `CurrentState` directly instead of going through `SetState` (see `docs/SoftwareUpdate.md`) — so there is nothing to derive executability from. A vendor machine from a NodeSet gets its causes through `StateMachineBuilder.For(...).WithCause(...)`, which the first change covers. Worth revisiting if the generator gains table emission. ## Notes for reviewers Two details behind the `ProgramStateMachineState` change: - The five methods are `OptionalPlaceholder` on `ProgramStateMachineType`, so the generated class has no typed properties for them. They are matched on browse name **without** the namespace index, because a subtype that redeclares them puts them in its own namespace — the Boiler model uses `1:Start`, not `0:Start`. - `NodeState.Create` rebuilds the child list from the type template, so a method added by hand before it is gone by the time `OnAfterCreate` runs. `WireCauseMethods(ISystemContext)` is public for that case. **Behaviour change worth flagging:** a `WithCause` against a machine whose `CauseMappings` lack that cause now reports the method non-executable where it previously reported executable. Such a call was already failing with `Bad_NotSupported` inside `DoCause`, so it fails earlier with the code Part 4 specifies rather than newly failing — but it is visible to clients that read the attribute. ## Related Issues - Fixes #4363 ## 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. ### Tests New: `StateMachineBuilderCauseExecutableTests` (definition mode, the opt-out, `OnCheckUserPermission` affecting `UserExecutable` only, suspended machines) and `ProgramStateMachineStateTests`, plus two cases added to the existing fluent-builder fixture. All read through `MethodState.ReadAttribute`, so a handler that is installed but never consulted fails the test. Run locally on **net10.0**: `Opc.Ua.Server.Tests` (4797), `Opc.Ua.Core.Tests` (4374), `Opc.Ua.Di.Tests`, `Opc.Ua.Features.Tests`, `Opc.Ua.Robotics.Intent.Tests` and the Boiler AOT tests all pass; `UA.slnx` builds with no new warnings. The .NET Framework legs were not run locally — leaving that checklist box to CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-01 18:31:36 +02:00
Marc Schier	e4c5016883	Fix reverse connect test timeout race (#4366) ... # Description Fix the flaky `ReverseConnect2Async(True,True,...)` failure reported by Azure DevOps as `BadConnectionClosed`. The fixture configured the server to close unclaimed reverse connections after 10 seconds while the client retained unmatched sockets for the 15-second default hold time. Under delayed CI execution, a newly registered waiter could therefore receive a socket the server had already closed. Configure the test client's hold time to 5 seconds, preserving a safe margin before the server timeout, and remove the `[Retry(2)]` workaround. The test now waits for a live reverse connection instead of masking stale-socket failures with a retry. ## Related Issues - [Azure DevOps build 17573 test result](https://opcfoundation.visualstudio.com/opcua-netstandard/_build/results?buildId=17573&view=ms.vss-test-web.build-test-results-tab&runId=1606445&resultId=100071&paneView=debug) - No GitHub issue. ## Validation - `ReverseConnect2Async`: 11 consecutive Release/net10.0 runs, 88 tests passed. - Complete `ReverseConnectTest` fixture: 50 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. - [ ] 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: 1b26a017-e529-402d-99f9-669be6057f48	2026-09-01 13:56:55 +00:00
Marc SchierandCopilot	7fe8b30cc3	Publish migration plugin marketplace and update NuGet dependencies (#4358) ... # Description - Replace the Azure DevOps continuous-preview feed with the GitHub Packages NuGet feed, including its classic PAT `read:packages` requirement, while retaining nuget.org for official public 2.0 preview releases. - Document one-shot MCP execution with `dotnet tool exec` and `dnx`. - Make this repository registerable as the `opcua-dotnet` Copilot marketplace and package `opcua-v20-migration` as an installable plugin. - Bundle the 15 thematic migration documents for offline plugin use and add a CI check that keeps the bundle and marketplace metadata synchronized. - Align the migration analyzer guidance with current master, including the new `UA0030` analyzer while retaining `UA0029` as a runtime-shim-only marker with reserved diagnostic metadata. - Restrict generated collection-shim type discovery to consumer source declarations before the exact `System.<Type>` / `Opc.Ua.<Type>` metadata fallback, with regression coverage for dependency-only metadata. - Update 52 unpinned NuGet package IDs across 60 central entries, plus two schema-test overrides, while preserving documented compatibility and security pins. ## Related Issues - Fixes #4330 ## Validation - `dotnet build UA.slnx -c Release --no-restore`: 0 warnings, 0 errors. - Migration analyzer tests across net472, net48, net8.0, net9.0, and net10.0: 795 passed, 0 failed, 0 skipped. - The migration plugin bundle, manifests, PowerShell, XML, links, and isolated marketplace installation validate successfully. - The direct and transitive NuGet vulnerability audit reports no known vulnerabilities. - A full solution test run was completed before merging current master. Its remaining failures were existing locale, path-length, GPU, shared-state, and flaky integration failures. Persistent cases reproduced on the pre-update commit, while isolated reruns cleared the dependency-relevant Core, GDS, Sessions, Subscriptions, and durable-queue failures. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0ecb0e9-e5ef-4b2f-9922-41a7d42b0372	2026-08-31 19:42:24 +00:00
Marc SchierandCopilot	4eccafbe2f	Remove AAS projects and samples (#4359) ... # Description Remove the Asset Administration Shell projects and samples from the mainline repository while preserving the complete implementation and history on the `aas` branch. This change: - removes the `Opc.Ua.Aas`, `Opc.Ua.Aas.Client`, `Opc.Ua.Aas.Server`, and `Opc.Ua.Aas.WoT` projects; - removes the AAS test project, AOT coverage, sample, and documentation; - removes AAS entries from `UA.slnx`, the expected package list, and related build configuration; - removes the now-unused `System.IO.Packaging` package version; and - updates remaining documentation and comments so they no longer advertise removed AAS functionality. The preservation branch `aas` points to the original `master` commit before this removal. ## Related Issues - None. ## Validation - `dotnet restore UA.slnx` - `dotnet build UA.slnx -c Release --no-restore` - Repository-wide sweep for dangling AAS project, package, file, and documentation references The full Release build completed with zero errors. It reported 31 existing analyzer warnings in files unrelated to this change. ## 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. (Not applicable: this removes a feature and its tests.) - [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>	2026-08-31 17:08:04 +00:00
Marc SchierandCopilot	8d3eccd41c	Fix concurrent channel reconnect certificate leaks (#4356) ... # Description Fix the intermittent one-certificate leak reported by the macOS Sessions suite. The leak had two concurrent ownership paths: 1. `ChannelEntry.OpenInitialAsync` could complete after a reconnect cycle had already installed a newer transport. The initial-open path overwrote `m_underlying`, orphaning the reconnect transport and its certificate handles. 2. `UaSCUaBinaryTransportChannel.ReconnectAsync` did not dispose a newly created inner channel when `ConnectAsync` failed. Initial-open cleanup could also be skipped when it reused the caller's already-cancelled token. The fix keeps the transport installed first, closes and disposes the losing candidate, disposes failed reconnect candidates, and performs failed initial-open cleanup independently of caller cancellation. This also adds opt-in Release allocation tracking for outstanding `Certificate` handles. Set `OPCUA_CERTIFICATE_LEAK_TRACKING=1` to include allocation stacks and NUnit suite attribution in leak failures. The contributor workflow is documented in `docs/DeveloperGuide.md`. ## Tests and verification - Added a deterministic `ChannelEntry` race test that orchestrates concurrent initial open and reconnect, then verifies the losing transport is closed and disposed. - Added deterministic UA-binary transport tests for failed reconnect certificate cleanup and retry after a cancelled initial open. - Pre-fix macOS diagnostic run reproduced 3 leaks in 15 full Sessions runs: https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/33329288604 - Post-fix targeted macOS run passed 50/50 exact reproducer executions: https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/33334875967 - Post-fix full macOS run passed 15/15 complete Sessions suites: https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/33335718790 - Full local net10 Sessions suite passed: 775 passed, 379 skipped, 1,154 total. - Focused ownership regressions passed on net10 and net48. - `Opc.Ua.Core` built warning-free across all supported target frameworks. - Final branch Build/Test, CodeQL, and NuGet workflows passed. The temporary reproduction workflow and trigger tags were removed after verification. ## Related Issues - Fixes #4338 ## 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. - [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 <223556219+Copilot@users.noreply.github.com>	2026-08-31 15:04:50 +02:00
romanettandClaude Opus 5	57db405446	Implement ServerStatusChanged on the GDS and push configuration clients (#4351) ... # Description `GlobalDiscoveryServerClient.ServerStatusChanged` and `ServerPushConfigurationClient.ServerStatusChanged` never fired. In 2.0 neither class raised them and both declarations sat behind `#pragma warning disable CS0067`; in 1.5.378 the GDS client had an empty `if (ServerStatusChanged != null) { }` marked `// TODO: implement` and the push client had no raiser at all. The XML docs claimed the opposite, so a consumer got a handler that was simply never called. Both clients now raise the event when the server state changes, and once for the state the first keep-alive reports after a connect. ## No subscription is created `KeepAliveEventArgs` already carries `Status`, `CurrentState` and `CurrentTime`. The session keep-alive reads `Server_ServerStatus_State` from the server on every interval — on both subscription engines — and sends the first one immediately after connect (`TimeSpan.Zero`). Both clients were already subscribed to `Session.KeepAlive`, so the state is available without any subscription of their own. Each client raises the event from its existing keep-alive handler. That means no subscription, no monitored item, no engine branching, no teardown lifecycle, and **no additional requests to the server**. An earlier revision of this PR did create a `Server_ServerStatus` subscription per connect. It was ~420 lines, needed separate classic and V2 paths, and hung the Gds test legs on macOS and ubuntu — the subscription was created inside `ConnectInternalAsync` under the client lock and the run wedged there deterministically (0 failures, a hang dump, remaining fixtures never executed). Reading the state off the keep-alive removes the code and the failure mode together. ## Handler type The events, and the (new in 2.0) interface members, use `EventHandler<ServerStatusChangedEventArgs>` rather than the `MonitoredItemNotificationEventHandler` the concrete classes declared in 1.5.378. That type is a classic-engine type whose first parameter is the concrete `MonitoredItem`, and `MonitoredItemNotificationEventArgs` has an `internal` constructor, so `Opc.Ua.Gds.Client.Common` cannot construct one. ```csharp client.ServerStatusChanged += (sender, e) => { ServerState state = e.CurrentState; }; ``` There is nothing to migrate behaviourally, since no 1.5.378 handler ever ran; code that merely subscribed needs a recompile. **Scope note:** the event reports the server *state* (matching its name) rather than the full `ServerStatusDataType`. A consumer needing `BuildInfo` or `SecondsTillShutdown` reads `Server_ServerStatus` directly. Happy to revisit if the richer payload is wanted. ## Note for reviewers: the blank-line hunks Removing `#pragma warning disable CS0067` unmasked **26 pre-existing RCS0009** violations in the two client files — that pragma had been suppressing the Roslynator analyzer for the whole file. Confirmed against a baseline worktree of `master`: deleting only the two pragma lines there reproduces all 26. Those blank-line-only hunks are required to keep the build green, not gratuitous reformatting. ## Related Issues - Fixes #4346 ## 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. (Behaviour is documented on the interface members; per review the migration-guide entry was dropped, since a never-raised event is a bug fix with nothing to migrate.) - [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. ### What I verified locally - `UA.slnx` builds clean (0 warnings, 0 errors). - The full `Opc.Ua.Gds.Tests` suite on **net10.0** — the one that hung in CI — now runs to completion: **1111 passed, 0 failed, 42 skipped, 7m37s**. It includes the two new integration tests asserting the event is raised after connect (`ClientTest`, all 13 `PushTest` fixtures). - I did not run the .NET Framework target locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-31 11:44:37 +00:00
romanettandClaude Fable 5	414957c88a	Reduce ISubscription: internalize the publish pipeline, delete the dead members (#4349) ... # Description Reduces the server subscription API surface per the measurements from issue #4185. ## ISubscription: 42 → 30 members (#4185) A call-site census of the 11 "publish-pipeline protocol" members plus `Publish` (all 68 `src/` projects, samples, tools, fuzzing, tests) found **zero callers outside `Opc.Ua.Server`** and split the members three ways: - **Deleted (4):** `ItemReadyToPublish` and `ItemNotificationsAvailable` were dead — zero callers repo-wide and implementation bodies commented out since before 1.5.378 — the obsolete parameterless `SessionClosed()` had zero callers, and `TransferSessionAsync` (the one-shot direct transfer, which bypassed the reservation protecting a transfer against a concurrently closing source session) lost its last caller once the manager always uses the claim/prepare/commit protocol. - **Internalized (8):** `PublishTimerExpired`, `Acknowledge`, `PublishTimeout`, `SubscriptionTransferred`, `AvailableSequenceNumbersForRetransmission`, `QueueOverflowHandler`, `SessionClosed(ISession)` and `Publish` move to `internal interface ISubscriptionPublishPipeline : ISubscription`, implemented **explicitly** by `Subscription` (the `ISubscriptionMonitoredItemLifecycle` shape) so they are unreachable through the public concrete class too. `Publish` goes with them deliberately — it is the same hazard class (consumes notifications, advances sequence numbers) with the same single production caller. - **Kept public (1):** `ResendData` drops out of the issue's list — its only caller is the `Server_ResendData` UA-method handler in `DiagnosticsNodeManager`, resolving via `TryGetSubscription` like `GetMonitoredItems` does. It is service-facing, not pipeline. ## The pipeline contract is complete: SessionPublishQueue traffics only in it `SessionPublishQueue` becomes `internal sealed` (zero consumers outside the assembly; `SentMessageQueue` already was) and works **solely with `ISubscriptionPublishPipeline`** — no `ISubscription` and no concrete `Subscription` anywhere in the file. To get there, the internal transfer-claim protocol (`TryBeginTransfer`, `PrepareSessionTransferAsync`, `CompleteTransfer`, `AbortTransfer`, `TryRestoreSessionAfterFailedTransfer`) moved from `internal` methods on the concrete class onto the internal interface, implemented explicitly. That completes the contract, so `TransferSubscriptionsAsync` loses its two-tier shape: the ~6 `subscription as Subscription` null-guard branches and the degraded `TransferSessionAsync` fallback for non-`Subscription` implementations are gone — one code path, always the claim/prepare/commit/rollback protocol, and `ISubscription.TransferSessionAsync` is deleted with it. Subscription creation fails with `Bad_InternalError` when a `CreateSubscription` override returns a type not derived from `Subscription`, instead of publishing partially — such implementations were already second-class (transfer required the concrete type before this change). This is the headline behavior change for exotic extenders and is documented prominently in `docs/migrate/2.0.x/sessions-subscriptions.md`. The durable-subscription open question in the issue is answered **no**: `Subscription/Persistence/` never references `ISubscription` (the store deals in `IStoredSubscription` DTOs; restore constructs the concrete type via `Subscription.RestoreAsync` inside the manager), so the internalization cannot break a store implementer. ## #4186: measured, prototyped, and deliberately kept as-is Body inspection showed only 4 of the 10 `ISubscriptionManager` wrappers are pure lookup-and-delegate (`Republish`, `SetTriggering`, `ModifyMonitoredItemsAsync`, `SetMonitoringModeAsync` — one caller each); the other 6 carry logic with no `ISubscription` substitute. A collapse of the four was implemented and then reverted on this branch: it traded ten uniform id-addressed members for a mixed dispatch shape in `StandardServer` without reducing what a reader must understand. **Every subscription service keeps going through `ISubscriptionManager`**; the wrapper bodies now resolve against the pipeline-typed registry. The decision is recorded in `plans/ServerRuntimeAnalysis.md` (open question 5). ## Compatibility treatment Removals are outright, per the documented no-`[Obsolete]`-shim policy for these interfaces (`docs/MigrationGuide.md` — re-adding an interface member would break every implementer; net48/netstandard2.0 have no default interface methods). In its place: - Migration analyzer **UA0030** (pipeline members + `SessionPublishQueue`, distinguishing deleted no-ops from internalized protocol), with tests, following the UA0024 syntax-based pattern. The transfer-claim members were never public in 1.5.378, so they need no migration story. - `docs/MigrationGuide.md#ua0030` section with before/after code; the `docs/migrate/2.0.x/sessions-subscriptions.md` section that told implementers to *add* `SessionClosed(ISession)` is replaced with the derive-from-`Subscription` story; the 2.0.x README symptom table routes the rule. - All 11 removed/internalized members existed in 1.5.378 (verified against `master378`), so each is documented as a 2.0 break. ## Tests Converted, not deleted. `Mock<ISubscription>` setups of pipeline members become `Mock<ISubscriptionPublishPipeline>` — Moq proxies the internal interface via a new `InternalsVisibleTo("DynamicProxyGenAssembly2")` (keyless; the SDK appends `$(PublicKey)` under `SignAssembly`, and signed pipelines do not run tests, matching the existing `Opc.Ua.Redundancy.Kubernetes` precedent). A test-side extension shim forwards the original member names (pipeline + transfer protocol) so the direct call sites on concrete `Subscription` instances compile unchanged. The transfer-race tests' typed reflection probes into `m_abandonedSubscriptions` follow the field's new type. - `Opc.Ua.Server.Tests` (net10.0): 4795 tests, 0 failures - `Opc.Ua.Subscriptions.Tests`, `Opc.Ua.Subscriptions.Durable.Tests`, `Opc.Ua.Subscriptions.Classic.Tests` (net10.0): green - `Opc.Ua.MigrationAnalyzer.Tests`: green incl. the new UA0030 tests - net48 gate: solution build green; subscription-area fixtures green (the known pre-existing local hosting-test environment failures reproduce identically on unmodified master) ## Related Issues - Fixes #4185 - #4186 was measured and prototyped on this branch and deliberately **not** taken (see the section above); the decision is recorded in `plans/ServerRuntimeAnalysis.md`. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. _(net10 suites fully green locally; net48 build green and subscription-area fixtures green — the remaining net48 hosting-test failures on my machine reproduce identically on unmodified master.)_ - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-31 10:53:51 +00:00
4 people	2bd4294863	[Server] Decouple service-call dispatch from MasterNodeManager lifecycle coordination (#4345) ... # Description Extracts OPC UA service-call dispatch out of `MasterNodeManager` into a dedicated internal `NodeManagerServiceDispatcher`, separating it from NodeManager lifecycle coordination as agreed in the #4094 review threads (r3649968439 / r3649998212). The 7,668-line class becomes four files with explicit, documented boundary contracts: - **`NodeManagerServiceDispatcher.cs`** (new, internal sealed) — the implementations of GetManagerHandle/Async, TranslateBrowsePaths, Browse/BrowseNext, Read, Write, Call, HistoryRead/HistoryUpdate, ConditionRefresh, all monitored-item operations (create/restore/modify/transfer/delete/set-mode), and per-request validation. Its constructor receives only the routing table, the server context, and a logger. Lifecycle mutation state (`m_dynamicMutationSemaphore`, `m_startupShutdownSemaphoreSlim`, retired-generation collections, preparing-manager tracking) stays `private` to `MasterNodeManager`, so the dispatcher cannot reach it — the boundary is enforced by the compiler, not convention. - **`MasterNodeManager.cs`** — now lifecycle coordination and shared infrastructure only: startup/shutdown, session notifications, namespace registration, all `IDynamicNodeManagerHost` / `ISyncNodeManagerMonitoredItemRecovery` implementations, the retired-generation notification-lease machinery, and commit/replace/rollback helpers. - **`MasterNodeManager.ServiceDispatch.cs`** (new partial) — the unchanged public service surface. Every public virtual entry point, protected helper, static validator, and obsolete wrapper keeps its exact signature, virtual-ness, and XML docs, delegating to the dispatcher. - **`MasterNodeManager.NodeManagement.cs`** (new partial) — the AddNodes/DeleteNodes/AddReferences/DeleteReferences service family plus NodeId-based reference plumbing. These sit between the two concerns by design: they dispatch per item but serialize address-space mutation against runtime lifecycle operations, and are the only service methods allowed to take the dynamic-mutation semaphore. The dispatcher's crossings back into `MasterNodeManager` are a small audited set: internal dispatch resolves handles through the owner's public virtual `GetManagerHandleAsync` (10 sites), so derived-class overrides keep affecting internal routing exactly as before; event paths consume the notification-dispatch lease accessors (5 sites, widened to internal); retired-generation drain is signalled through a new one-line internal helper instead of touching the observer field (2 sites); and the browse continuation-point limit is read live per call (2 sites). ## Compatibility - `IMasterNodeManager` is untouched; no public or protected member changes signature or loses `virtual`. - The `MasterNodeManagerWithLimits` test-framework subclass (overrides `BrowseAsync`, uses `ValidatePermissionsAsync`, the per-item `BrowseAsync`, `UpdateDiagnostics`, `Server`) passes unchanged. - Reflection-dependent tests keep working: `m_maxContinuationPointsPerBrowse` remains a private field on the type and is read live on every call; `PreHydrateMonitoredItemQueuesAsync` remains as a private delegating method. - Reviewable API-surface deltas, both additive: six `protected static` validators became `protected internal static` (same pattern as the existing `ValidateRolePermissions`) so the dispatcher can call them, and the three notification-lease nested types went `private` → `internal`. Reviewing tip: `git diff --color-moved=dimmed-zebra` renders the two move commits as almost entirely dimmed blocks; the non-move edits are confined to the audited call sites above and the delegation stubs. Grepping the dispatcher for `IDynamicNodeManagerHost|m_dynamicMutationSemaphore|m_startupShutdownSemaphoreSlim|m_retiredGeneration|m_preparingNodeManagers` matches only its own boundary-contract doc comment. ## Verification - Full `UA.slnx` build with 0 errors; `Opc.Ua.Server` itself builds with 0 warnings. - Full `Opc.Ua.Server.Tests` on net10.0: 4707 passed, 0 failed (5 pre-existing skips). - Targeted: MasterNodeManager/Deterministic/NodeManagement 189/189; NodeManagerLifecycle/MonitoredItemLifecycle/MonitoredItem 406/406; `ContinuationPointInBatch` (subclass contract) 28/28. - `docs/NodeManagers.md` documents the new internal split. ## Related Issues - Fixes #4098 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. _(pure refactoring; behavior is pinned by the existing MasterNodeManager, deterministic, node-management, lifecycle, and monitored-item suites, which all pass unchanged)_ - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. _(full server suite green on net10.0; .NET Framework run pending CI)_ - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>	2026-08-31 12:04:47 +02:00
romanettandClaude Opus 5	052576115d	Fix ManagedSession.CreateAsync returning an unconnected session when the initial connect fails (#4350) ... # Description When the **initial** connect of a `ManagedSession` failed, `CreateAsync` did not throw. It returned a session with no inner `Session` behind it, so every subsequent member threw `BadNotConnected` and the real error — `BadCertificateUntrusted` in the reported case — was swallowed. **Mechanism.** A failed initial connect moves the state machine `Connecting` → `Reconnecting`, which calls `ManagedSession.HandleReconnectAsync`. That method skipped its entire body when `m_session` was `null` — exactly the state after a failed initial connect — and fell through to `return ServiceResult.Good`. The state machine read that as a successful reconnect, transitioned to `Connected`, released the `CreateAsync` waiter, and handed back a session that was never connected. The logs show it plainly: a "Reconnected" line immediately after "Reconnecting" with no service call in between. The terminal path was broken as well: once the reconnect policy **and** the failover were exhausted the machine settled in `Disconnected` and nothing ever set the connected event, so `WaitForConnectedAsync` — and with it `CreateAsync` — waited forever rather than surfacing the failure. **Fix.** - `ManagedSession.HandleReconnectAsync` reads the inner session once under the service lock and, when there is none, runs a full connect (`HandleConnectAsync`) instead of reporting success. The reconnect policy now genuinely retries the initial connect. - `ConnectionStateMachine` tracks the error of the last failed connect/reconnect attempt and sets a new *settled* event when the connect cycle ends — either by connecting or by giving up (`Disconnected` after a failed failover, or `Closing`/`Closed`). `WaitForConnectedAsync` waits on that and throws a `ServiceResultException` carrying the last attempt's error when the connection was abandoned. A failover failure only supplies that error if no attempt recorded one, so the `BadNotSupported` returned by a failover with no redundant server configured never masks the actual cause. `CreateAsync` can no longer hand back a session that reports connected with `m_session == null`. ## Behavior change worth reviewing A permanently failing initial connect no longer returns instantly: `CreateAsync` retries under the configured policy and throws only once `MaxRetries` / `MaxTotalReconnectTime` and failover are exhausted (default budget 5 minutes). Callers wanting fail-fast cap the policy or pass a cancellation token — documented in `docs/Sessions.md`. No status code is treated as non-retryable. Classifying e.g. `BadCertificateUntrusted` as fatal would break the very scenario in the issue, where the connect can start succeeding once an operator trusts the certificate. ## Tests - `ConnectionStateMachineTests.WaitForConnectedAsyncThrowsOriginalErrorWhenConnectGivesUp` — the original error survives the failover's `BadNotSupported`. - `ConnectionStateMachineTests.WaitForConnectedAsyncThrowsWhenClosedWhileConnecting` — a close while connecting releases the waiter instead of hanging. - `ManagedSessionTests.CreateAsyncThrowsInitialConnectErrorInsteadOfReturningUnconnectedSession` — asserts the original status propagates out of `CreateAsync` and that the connect was attempted more than once. - `WssOpenApiIntegrationTests.ManagedSessionOverWssOpenApiBearerRejectedWhenNoBearerAuthRegistered` relied on the old behavior (it asserted `Connected == false` on the session returned by a failed connect). Rewritten to bound the retry policy and assert that `ConnectAsync` throws; the fail-closed HTTP 401 property it guards is unchanged. Suites run locally on net10.0, all green: `Opc.Ua.Client.Tests` (2155), `Opc.Ua.Sessions.Tests` (775), `Opc.Ua.Gds.Tests` (1096), `Opc.Ua.Client.ComplexTypes.Tests` (2773), `Opc.Ua.Redundancy.Client.Tests` (122). No new build or analyzer warnings. ## Related Issues - Fixes #4347 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-31 12:02:02 +02:00
6 people	14fba2d120	Fix instance TypeDefinition across dependency ModelDesign boundaries (#4355) ... # Description The model source generator crashed with `MODELGEN003` / `NullReferenceException` whenever a `ModelDesign` instantiated an `ObjectType` (or `VariableType`) declared in a **dependency** design file, as soon as that type had children. The sibling of #4332 on the instance-declaration path: #4335 linked dependency *data types*, but an *instance* whose `TypeDefinition` lives in another design still crashed in `NodeStateGenerator` on the type's inherited children. Root cause: dependency design files load with `validateDictionary: false`, so `ValidateInstance` / `ValidateType` never ran on them and their types' children kept `TypeDefinitionNode` / `DataTypeNode` at `null`. When the target instance merged those children into its hierarchy (`BuildInstanceHierarchy` re-walks the dependency type's `Children.Items`), the null references were copied along, and `GetNodeStateClassName` dereferenced them while building the node state factory list. Changes: - A new best-effort `LinkDependencyInstances` pass in `ModelDesignValidator` links dependency-design instances the way `ValidateInstance` / `ValidateType` do for the target model: the `TypeDefinitionNode` / `DataTypeNode` of the types' children and standalone instances (recursively), variable `DataType` / `ValueRank` inheritance from the variable type, `DefaultValue` decoding, `VariableTypeDesign.DataTypeNode` restrictions, and method arguments — including creation of the `InputArguments` / `OutputArguments` argument properties so method children materialise into the target hierarchy exactly like local ones. Resolution is best effort (unresolvable references stay `null`) and NodeSet2 dependencies remain excluded, consistent with `LinkDependencyDataTypes`. - The pass runs alongside `LinkDependencyDataTypes`: for upstream dependencies after all of them are loaded and before the target validates (so the hierarchy merge picks up the links), and for downstream dependencies after they load. - `docs/ModelDependencies.md` documents the instance linking. Tests (all verified to fail with the exact reported NRE stack before the fix): - `CrossModelInstanceTypeDefinitionTests` (Core, 4 tests): the issue's minimal repro end to end; an equivalence test asserting the cross-model instance emits the **identical node-state factory set** as a locally declared type (this is the issue's stated expectation, and it drove the method-argument-property part of the fix); the flipped upstream/downstream ordering; and validator-level assertions that the merged hierarchy carries linked `TypeDefinitionNode` / `DataTypeNode` references, including the data type inherited from a dependency `VariableType`. - `CrossModelInstanceTypeDefinitionGeneratorTests` (Roslyn driver, 2 tests): the issue's csproj repro shape — both `AdditionalFiles` orderings, asserting no diagnostics and that the combined generated output of both models **compiles** (the instance factories reference the dependency model's typed state classes). Verified locally on net10.0: `Opc.Ua.SourceGeneration.Core.Tests` 3800 passed, `Opc.Ua.SourceGeneration.Tests` 141 passed, `Opc.Ua.SourceGeneration.Stack.Tests` 94 passed; 0-warning Release build of `Opc.Ua.SourceGeneration.Core` (all TFMs incl. netstandard2.0); `dotnet format whitespace --verify-no-changes` clean. This removes the last generator blocker for source generating the instance model of the `Workshop/DataTypes` sample (OPCFoundation/UA-.NETStandard-Samples#814): its `ParkingLot` / `DriverOfTheMonth` are instances of types declared in the other design file. ## Related Issues - Fixes #4353 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-31 09:47:08 +00:00
romanettandClaude Fable 5	24ba4855e6	Fix [DataType] generator Clone() dropping every inherited field of a derived structure (#4354) ... # Description The `[DataType]` source generator emitted a `Clone()` that created a fresh instance and copied only the fields declared on the annotated type itself, so every field inherited from an `IEncodeable` base was silently dropped. Since the server clones node values while loading an address space, a derived structure served by a node manager came back out of a Read with its inherited fields empty, even though encode/decode was correct. This PR makes the `[DataType]` path emit the same `MemberwiseClone`-based shape as the model-based generator: ```csharp /// <inheritdoc/> public override object Clone() { return (DerivedStruct)this.MemberwiseClone(); } /// <inheritdoc/> public new object MemberwiseClone() { DerivedStruct clone = (DerivedStruct)base.MemberwiseClone(); // deep copy of the own reference-typed fields return clone; } ``` `Clone()` delegates to a generated `public new object MemberwiseClone()` that starts from `base.MemberwiseClone()` — a shallow copy of the whole object, inherited fields included — and then deep copies the type's own reference-typed fields (encodeables, `DataValue`, `Variant`, `ExtensionObject`). Derived classes hide `MemberwiseClone` and chain into the base implementation, so deep-copy semantics compose across the hierarchy, including across assemblies into model-generated base types (the `BicycleType` scenario that surfaced this). The two generator paths now agree. Because the clone now starts from a full shallow copy (`base.MemberwiseClone()` for classes, `with { }` for records), the per-field clone emitter only writes the deep-copy assignments; the redundant per-field plain copies are gone. This also fixes a latent side bug in the class path: init-only simple fields were silently dropped by the fresh-instance clone as well. Records were already correct (`with { }` carries every field) and are unchanged in shape. Tests: - `GenerateClassCloneUsesMemberwiseClone` / `GenerateDerivedClassCloneCarriesInheritedState` assert the emitted shape for root and derived classes. - `DerivedClassCloneKeepsInheritedFields` runs the exact repro from the issue through the Roslyn driver, emits the assembly, and executes `Clone()` at runtime to prove the inherited field survives (verified on net10.0 and net48). Verified locally: `Opc.Ua.SourceGeneration.Core.Tests` (3798 passed), `Opc.Ua.SourceGeneration.Tests` (140 passed), `Opc.Ua.SourceGeneration.Stack.Tests` (94 passed), and `Opc.Ua.Client.Tests` (2161 passed, exercises the regenerated in-tree `[DataType]` record types). The in-tree `[DataType]` consumers (Opc.Ua.Client, both GDS common projects) regenerate and build without warnings. ## Related Issues - Fixes #4352 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-30 22:10:27 +00:00
romanettandClaude Opus 5	16c54e52d9	Keep application certificates which do not name a CertificateType (#4348) ... # Description `SecurityConfiguration.ApplicationCertificates`'s setter removed every identifier whose certificate type is not supported, and `CertificateIdentifier.CertificateType` defaults to unset. An otherwise correct identifier passed to the recommended `AddSecurityConfiguration(ArrayOf<CertificateIdentifier> certIdList, ...)` overload was therefore dropped silently, and validation then failed with `ApplicationCertificate must be specified.` — pointing at the wrong cause. The obsolete `AddSecurityConfiguration(string subjectName, ...)` overload escaped this because it assigns the legacy singular `ApplicationCertificate` property, whose setter fills in `RsaSha256ApplicationCertificateType` itself. The result: the replacement advertised by the `[Obsolete]` message could not be used without also setting `CertificateType`, which that message does not mention. **Changes:** - `SecurityConfiguration.ApplicationCertificates` (`src/Opc.Ua.Core/Schema/ApplicationConfiguration.cs`) now treats an unset `CertificateType` as the default RSA-SHA256 application certificate type instead of dropping the entry — the same assumption the legacy single `ApplicationCertificate` element carries, so both paths agree on what a valid `CertificateIdentifier` is. Fixing the setter also covers callers that assign `ApplicationCertificates` directly, not just the builder overload. - Identifiers dropped for a genuinely unsupported type are no longer discarded without a trace. They are remembered and reported by `SecurityConfiguration.Validate`, which has an `ITelemetryContext` — this replaces the `// TODO: Log when ITelemetry instance is available` that was exactly this bug: - a warning naming the rejected certificate types is logged whenever any were filtered out; - when the collection ends up empty *because* of the filtering, the error now reads `No supported application certificate configured: N certificate identifier(s) were rejected because their CertificateType is not supported (...)` instead of `ApplicationCertificate must be specified.`. The original message is kept for the genuinely-empty case. **Notes for reviewers:** - Behaviour is unchanged for every configuration that already names a certificate type; all shipped and test config XMLs set `CertificateTypeString`. - `FindApplicationCertificateAsync` already had a fallback treating a null certificate type as RsaSha256, and `ApplicationInstance` fills in the type from the created/validated certificate when it is null — defaulting in the setter is consistent with both and yields the same RSA certificate. - The unsupported-type filter itself is untouched; only unset types are now kept. ## Related Issues - Fixes #4315 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - `SecurityConfigurationTests`: an untyped identifier survives as RsaSha256 and contributes `Basic256Sha256` to `SupportedSecurityPolicies`; an unsupported type (`EccCurve25519ApplicationCertificateType`) is removed and reported by `Validate` with `BadConfigurationError`; a supported entry survives alongside an unsupported sibling without failing validation. - `ApplicationConfigurationBuilderTests`: end-to-end reproduction from the issue, building a client through `AddSecurityConfiguration(ArrayOf<CertificateIdentifier>, pkiRoot)` + `CreateAsync()`. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - Ran on net10.0: `Opc.Ua.Configuration.Tests` 225/225 passed, `Opc.Ua.Core.Tests` 4368 passed / 86 skipped / 0 failed. The full solution across .NET Framework was not run locally — relying on CI for that. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-30 20:27:20 +00:00
3 people	52df519e50	Expose the client configuration provider and add an opt-in eager load on host start (#4343) ... # Description `AddClient(configurationFile)` (#4328) loads the supplied configuration XML lazily on the first DI session connect, and the `ClientSuppliedConfigurationProvider` behind it was reachable only through internals. Hosted clients whose user interface needs the `ApplicationConfiguration` **before any session exists** therefore had no public way to trigger the load or to read the configuration back — the concrete blocker for moving the 14 WinForms sample clients and the GDS client off the classic eager `ApplicationInstance` path (OPCFoundation/UA-.NETStandard-Samples#794 / OPCFoundation/UA-.NETStandard-Samples#808). Two small additions, exactly as designed in #4340: **1. `OpcUaClientOptions.ConfigurationProvider` is now public** (`get`; the setter stays `internal`). It is already typed as the public `IOpcUaApplicationConfigurationProvider`, so consumers get everything they need with **no new types**: `GetAsync(ct)` as an explicit load trigger that does not connect a session, `Configuration` for the loaded document, and `Application` (`IApplicationInstance`) for the applications that manage their own instance certificate. Registering the supplied provider as `IOpcUaApplicationConfigurationProvider` in DI was deliberately *not* done — `OpcUaServerHostedService` binds to the last registered interface when it has no document of its own, so in a combined client+server host a client-only document would silently capture the server. **2. `OpcUaClientOptions.LoadConfigurationOnStart` — an opt-in eager load for hosted applications.** It registers an internal `ClientConfigurationLoaderHostedService` that awaits `GetAsync` during host start, the client-side twin of `ReverseConnectManagerHostedService`. When `host.StartAsync()` returns, the document is loaded and validated, the application-instance certificate is ensured, `Configuration` is published onto the resolved options (exactly as the connect path does), and a load failure fails the host start instead of surfacing on the first connect. It is registered ahead of the reverse-connect hosted service, so the configuration is complete before anything consumes it. The flag applies to every provider source — a document supplied via `ConfigurationFile` / `ConfigurationStream` and a shared `ConfigureApplication(...)` provider alike — and is a no-op for an explicit `Configuration`, which is already complete. It is bindable from `OpcUa:Client:LoadConfigurationOnStart`. Applications without a Generic Host call `GetAsync` on the provider directly instead; both routes share the provider's single-flight load, so the configuration is never loaded twice. A WinForms client then becomes: ```csharp services.AddOpcUa().AddClient("MyClient.Config.xml", o => o.LoadConfigurationOnStart = true); await host.StartAsync(); ApplicationConfiguration configuration = host.Services .GetRequiredService<OpcUaClientOptions>() .ConfigurationProvider!.Configuration; ``` **Reviewer note:** because the flag decides a *service registration*, it must be set inside the `AddClient(...)` callback or bound from the configuration section — setting it on the resolved options afterwards has no effect. This is called out in the XML docs and in `DependencyInjection.md`. **Coverage:** three tests added to `ClientConfigurationFileTests` for the supplied-document path (eager load on start including the certificate bootstrap and `Application` surface, load failure surfacing on start, no hosted service registered without the flag) and a new `ClientEagerConfigurationLoadTests` fixture for the provider-agnostic behavior (shared `ConfigureApplication(...)` provider awaited exactly once, explicit-`Configuration` no-op, host-startup-token cancellation aborting the load, configuration-section binding, and an end-to-end `host.StartAsync()` test). All 331 `ClientBuilder` tests pass (8 new), as do the 185 server hosting tests including the combined client+server fixtures. `Opc.Ua.Client` builds warning-free on all target frameworks; the test project builds clean on net10.0 and net472. **Docs:** new "Client: loading the configuration eagerly" section in `docs/DependencyInjection.md` and a pointer in the `Opc.Ua.Client` NuGet README. With these in place the sample clients can switch in a follow-up to OPCFoundation/UA-.NETStandard-Samples#808. ## Related Issues - Fixes #4340 - Builds on #4328 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>	2026-08-30 20:50:46 +02:00
romanettandClaude Opus 5	ad6430f560	Fix transient PublishWorkerCount overshoot in the publish controller (#4344) ... Fixes #4339. ## Cause `PublishControllerAsync` sized the worker pool at the **top** of its loop, before its first `await`. The `SubscriptionManager` constructor calls the controller directly, so that first resize ran *synchronously inside the constructor* — with `MinPublishWorkerCount`/`MaxPublishWorkerCount` still at their defaults (2 / 15), because owners configure them afterwards. A session that already reports subscriptions — a classic, session-owned subscription counts through `ISubscriptionManagerContext.SessionSubscriptionCount` — therefore had `1` clamped **up** to the default minimum of 2, spun up two workers in the constructor, and published that overshoot through `PublishWorkerCount`. The configured `Min = Max = 1` then shrank the pool back asynchronously. Confirmed with a probe before changing anything: ``` PROBE right after ctor: 2 ``` which reproduces the issue's `Worker #0 STARTED / #0 PAUSED / #1 STARTED` log exactly. `ClassicSessionSubscriptionStartsPublishWorkerAsync` pins the pool to one worker and samples the property directly, so it failed whenever it read it before the controller had shrunk the pool back. ## Fix The control loop now waits first and sizes the pool afterwards: ``` wait -> reap exited workers -> resize -> publish the count ``` The pool is only ever sized in response to a signal or a worker exit, never from unconfigured defaults, and the count is assigned once the pool has settled — so a worker that has exited is never counted alongside its replacement (the second half of the issue's *Expected Behavior*). Two supporting changes were required: - **Publish workers now start on the thread pool.** The controller creates them inline, and the worker loop only yields where it awaits something not already complete. With the resize moved behind the wait, workers are typically created while publishing is already running, which made that starvation reachable: I hit it while testing — 430k publishes in 2 s with `PublishControlCycles` stuck at 1 and `PublishWorkerCount` stuck at 0. The same starvation would hang `DisposeAsync`, which awaits the controller. - **`Resume()`, and leaving publishing quiescence, now signal the publish control**, so the pool is sized promptly once publishing starts rather than waiting for an unrelated event. ## Tests - `ClassicSessionSubscriptionStartsPublishWorkerAsync` now settles the pool through the existing `WaitForPublishWorkerCountAsync` helper instead of sampling the property bare. - New `PublishWorkerPoolIsNeverSizedFromDefaultsBeforeConfigurationAsync` pins the constructor behaviour (count stays 0 while the defaults are in force) and asserts the count never leaves 1 while workers publish. ## Verification - `Opc.Ua.Client.Tests`: 2151/2151 pass. - `Opc.Ua.Subscriptions.Tests`: 619 passed, 6 skipped, 0 failed (22 m 18 s). - The two affected tests stressed 15x in a row: green. - `dotnet format whitespace --verify-no-changes` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-30 17:40:48 +00:00
7 people	766c5558e8	Fix structure subtyping across dependency ModelDesign boundaries (#4335) ... # Description The model source generator crashed whenever a `ModelDesign` declared a structure that subtypes a structure from **another** design file (`MODELGEN003` with a `NullReferenceException`), and the cross-assembly `[assembly: ModelDependency]` flow failed even earlier with `The BaseType reference ... is not the expected type: DataTypeDesign`. Structure inheritance across model boundaries — the primary use case of companion-spec layering — could not be source-generated at all. Three root causes, all in the dependency-loading path of the validator: 1. **Dependency design files were never linked.** They load with `validateDictionary: false`, so their `DataTypeDesign`s kept `BasicDataType` at the enum default (`Boolean`) and their fields' `DataTypeNode` stayed `null`. `BinarySchemaGenerator`'s inherited-field merge then dereferenced the null `DataTypeNode`, and `XmlSchemaGenerator` / `DataTypeGenerator` silently emitted `xs:boolean` / `bool` from the default classification. 2. **Dependencies that import the target crashed at load.** Each generation group receives every design file of the compilation as a dependency, including downstream models that import the group's target — those loaded *before* the target they import and failed resolving their `BaseType`. This produced the confusing `AdditionalFiles`-order-dependent failure modes. 3. **The payload flow never reached ModelDesign targets.** `ModelCompilation` supplied `referencedDependencies` only to the NodeSet2 pass, never to the design-file pass, and payload-materialised data types lacked the classification and field links needed to serve as base types of local structures. Changes: - `ModelDesignValidator.ValidateModel` loads dependencies in two phases around the target: upstream models first (topological, referenced models before their referrers), then the target, then models that import the target. - A new `LinkDependencyDataTypes` pass classifies dependency data types (`IsStructure` / `IsEnumeration` / `BasicDataType`) and resolves field `DataTypeNode` / `Parent` best effort. It runs for payload-imported declarations and upstream design-file dependencies before target validation, and for downstream dependencies after they load. NodeSet2 dependencies are excluded — `NodeSetToModelDesign` already links and classifies them with its own semantics. - Payloads for models that are backed by a design file in the compilation are skipped — the explicit file is authoritative (previously, importing both crashed with duplicate symbolic ids). - Payload-materialised data types honor the payload's `IsEnumeration` flag. - The design-file `GenerateCode` pass now receives the referenced assemblies' dependency payloads (`ModelCompilation` + new optional `referencedDependencies` parameter). - `docs/ModelDependencies.md` updated accordingly. Tests (all verified to fail with the exact reported errors before the fix): - `CrossModelStructureSubtypeTests` (Core, 6 tests): the issue's minimal repro end to end — asserting the emitted `.bsd` carries inherited fields with correct types and `SourceType`, the `.xsd` extends the dependency structure (not `xs:boolean`), and the generated C# derives correctly — plus the reversed target/dependency order, both payload flows at generation and validator level, and a three-level chain (target → payload structure → design-file structure). - `CrossModelStructureSubtypeGeneratorTests` (Roslyn driver, 3 tests): both `AdditionalFiles` orderings compiling the generated output, and the full cross-assembly flow (producer assembly generated from the upstream design; consumer resolves the base structure from the reference's payload without re-emitting it). Verified locally on net10.0: `Opc.Ua.SourceGeneration.Core.Tests` 3786 passed, `Opc.Ua.SourceGeneration.Tests` 135 passed, `Opc.Ua.SourceGeneration.Stack.Tests` 94 passed; 0-warning Release builds of `Opc.Ua.SourceGeneration.Core` (all TFMs incl. netstandard2.0), both Roslyn generator variants, Stack and Tester. This unblocks the `[NodeManager]` migration of the DataTypes workshop sample (OPCFoundation/UA-.NETStandard-Samples#757). ## Related Issues - Fixes #4332 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-08-30 17:22:34 +00:00
4 people	1f0125c385	Fix reverse connect hold time when several Servers share one listener (#4342) ... # Description Fixes the reverse connect problem reported in #3985 on `master`. #4009 and #4059 addressed the listener startup diagnostics and the async lifecycle, but the actual hold time defect described in [mrsuciu's comment on the issue](https://github.com/OPCFoundation/UA-.NETStandard/issues/3985) survived both refactors unchanged. Symptom: a Client which uses one reverse connect listener port for several Servers connects to the first Server, and every following `WaitForConnectionAsync` times out. A separate listener port per Server was the only workaround. ## Root cause - Servers which send `ReverseHello` before the application registered a waiting connection for them are held in `OnConnectionWaitingAsync` for `HoldTime`. - All held connections share a single `CancellationTokenSource`, so `RegisterWaitingConnection`/`WaitForConnectionAsync` wakes up **every** held connection, not only the one it was registered for. - The hold loop was `while (!matched) { ... break; }` with an unconditional `break`, so each woken connection got exactly one re-match attempt. The connections which did not match the new registration fell out of the loop unaccepted, although most of their own hold time was left. - An unaccepted connection is force faulted by the listener (`TcpReverseConnectChannel`, "The reverse connection was rejected by the client"), so that Server is dropped and only returns on its next `ReverseHello` interval, long after the Client timed out waiting for it. Separate ports avoid the shared wakeup path, which is exactly why that workaround succeeds. ## Changes - `OnConnectionWaitingAsync` re-arms the hold after a wakeup: it re-matches, and if the registration which caused the wakeup was for another Server the connection keeps waiting for the remainder of its own hold time. The loop is still bounded by `HoldTime`, so a Server which is never registered for is rejected exactly as before. - New `IsCallbackDrainPending()` check before the hold is re-armed. `DrainConnectionCallbacksAsync` cancels the hold token without renewing it, so a re-armed hold would spin on the already cancelled token until the hold time expires and would block the drain until then. With the check a held callback releases its transport immediately, which keeps the existing `StopDrainsHeldConnectionCallbackBeforeListenerClose` and `DisposeDrainsHeldConnectionCallbackBeforeListenerDisposal` guarantees intact. - `docs/ReverseConnect.md`: document the `HoldTime` semantics in the "Sharing a listener across multiple Servers" section. No public API change. ## Validation - New `HeldConnectionSurvivesRegistrationForAnotherServer` test in `ReverseConnectManagerLifecycleTests` drives two held callbacks through `InvokeConnectionWaitingForTest` and proves the second Server keeps its connection when the first Server is registered. Verified that it fails against the previous behaviour with "the reverse connection of the second Server was released before its hold time expired". - `FullyQualifiedName~ReverseConnect` (183 tests, including `ReverseConnectManagerTests`, `ReverseConnectManagerLifecycleTests` and `ReverseConnectHostTests`): all pass on net10.0 and net48. ## Related Issues - Relates to #3985. - The same fix for the LTS branch is #4341 (base `master378`). ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-08-30 16:38:07 +00:00
4 people	90a85eda13	Fix V2 managed subscription keep-alive timer clamping causing false PublishingStopped flapping (#4324) ... # Description V2 managed subscriptions were clamping computed keep-alive intervals to `Timeout.InfiniteTimeSpan` (`-1ms`), then immediately flooring to `1s`, which caused continuous false `PublishState.Stopped → Recovered` flapping during normal publish gaps. This change corrects interval clamping so the watchdog uses server-revised keep-alive timing instead of a forced 1-second cadence. - **Root cause correction (V2 `Subscription.StartKeepAliveTimer`)** - Replaced invalid upper-bound check against `Timeout.InfiniteTimeSpan` with a real timer max bound. - Added `s_maxKeepAliveTimerInterval = TimeSpan.FromMilliseconds(uint.MaxValue - 1u)` and clamp to that value. - **Behavioral impact** - Preserves expected keep-alive windows derived from `CurrentPublishingInterval * (CurrentKeepAliveCount + 1)`. - Prevents false outage signaling on normal inter-publish gaps. - **Regression coverage** - Added a focused test asserting revised values (e.g., `2800ms`, keep-alive count `30`) produce the expected interval and do not collapse to `1s`. ```csharp if (m_keepAliveInterval > s_maxKeepAliveTimerInterval) { m_keepAliveInterval = s_maxKeepAliveTimerInterval; } ``` ## Related Issues - OPCFoundation/UA-.NETStandard#4323 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #4323 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-08-30 14:47:50 +00:00
5 people	651beaee1e	Fully support AliasNames in ConsoleReferenceServer (#4012) (#4321) ... # Description The CTT AliasName conformance units skip or fail against the ConsoleReferenceServer. `AliasName Base` skips with *"No instance of AliasNameType has been found in the address space"*; `Category Tags`, `Category Topics` and `Hierarchy` fail. Part 17 support landed in #3779, but aliases live only inside the `IAliasNameStore` — nothing is browsable, no `AliasNameCategoryType` is nested under another, and the Topics aliases point at the Server object rather than at a `PublishedDataSetType` instance. Part 17 §6.2 clients — the CTT among them — discover aliases by browsing, not only by calling `FindAlias`. This PR materializes the store contents as real nodes and fills the three gaps. ### SDK — `Opc.Ua.Server` * **`DiagnosticsNodeManager.MaterializeRegisteredAliasNameNodesAsync`** (new, opt-in) walks every registered `IAliasNameStore` and creates one `AliasNameType` instance per alias — BrowseName carrying the alias name, `AliasFor` references to its targets, and the inverse `HasAlias` reference on each local target (via `externalReferences`, since targets usually live in another node manager) — plus an `AliasNameCategoryType` instance for every store category the standard NodeSet does not already ship. Categories whose NodeId belongs to another node manager are skipped, so an `AliasNameNodeManager` that also registers its store with the server-wide registry does not get its NodeIds claimed twice. Opt-in: servers that only need the well-known `FindAlias` to answer from their store pay nothing. The nodes are a snapshot taken at address-space creation — later mutations change what `FindAlias` returns but do not add or remove nodes. * **Optional Part 17 methods on the well-known categories.** The same pass instantiates whatever a category declares through `AliasNameCapabilities`: `FindAliasVerbose` (§6.3.3), `AddAliasesToCategory` (§6.3.4), `DeleteAliasesFromCategory` (§6.3.5) and `LastChange` (§6.3.1). Mutations require a `SecurityAdmin` caller on a `SignAndEncrypt` channel; the gate previously private to `AliasNameNodeManager` moved to `AliasNameMethodDispatcher` so both paths share one implementation. * **`LastChange`** is now tracked per category rather than only for the standard `Aliases` node, and the duplicated seeding logic collapsed into one helper. ### Reference server * A nested **`Devices`** category under `TagVariables` holds `Pump1_Status` and `Heater_Power`. `FindAlias` on `TagVariables` still returns all six names through subtree recursion, while `FindAlias` on `Devices` returns exactly two — which is what the Hierarchy unit checks. * `ReferenceServerConfigurationNodeManager` creates three **`PublishedDataSet` instances** under `PublishedDataSets` (`i=17371`) and the Topics aliases now target them, satisfying the *"is a Dataset"* check. These are address-space instances only — no PubSub runtime is added, and the CTT only inspects the target's type definition. ## Note for reviewers — the reserved NodeIds The optional children are created at the NodeIds the OPC Foundation reserves for them (`Aliases.FindAliasVerbose` = `i=24054`, `TagVariables.LastChange` = `i=32854`, …). Those identifiers are allocated in `StandardTypes.csv` but are **not** declared in the ModelDesign and **not** present in the published NodeSet, because the standard address space does not instantiate optional children. The source generator therefore emits no parent-to-instance mapping and no `MethodIds` constants for them, so `DiagnosticsNodeManager.ReservedChildIds` supplies the nine method ids and the two missing `LastChange` ids explicitly, each traceable to its registry row. The 18 argument-property ids do have generated `VariableIds` constants and are referenced through those. I deliberately did **not** "fix" this by declaring the children in `tools/Opc.Ua.SourceGeneration.Core/Design/StandardTypes.xml`: that file is a verbatim copy of upstream `UA-ModelCompiler/Design.v105/StandardTypes.xml` (the only differences today are upstream being newer), so a local edit is lost on the next sync and the NodeIds would silently revert to factory-minted ones. `docs/AliasNames.md` records this. Worth raising upstream separately: `StandardTypes.csv` allocates `Aliases_FindAliasVerbose` and siblings while the ModelDesign never declares those children, so the ModelCompiler cannot emit ids it has itself reserved. If that is ever reconciled, `ReservedChildIds` becomes redundant and deletes cleanly. ## Related Issues - Fixes #4012 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. — **not done: verified on net8.0 only** (Server 4710, InformationModel 1099, Core 4443, plus the client and features alias suites; AOT build clean). Needs a run against net framework and net10 before this leaves draft. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ### Tests `tests/Opc.Ua.InformationModel.Tests/Aliasname*Tests.cs` mirror the four CTT conformance units and no longer `Assert.Ignore` where the CTT reported failures — the alias suite is 37 tests with no skips. New `AliasnameOptionalMethodsTests` covers the optional methods, their reserved NodeIds, the anonymous-caller denial, an add/find/delete round trip and `LastChange` advancing on mutation. One pre-existing test changed premise: `EnumerateSubCategoriesAsyncOnLeafReturnsEmptyAsync` asserted that `TagVariables` has no sub-categories, which this PR deliberately makes false. It now targets `Topics` (still a leaf) and a new test guards the nesting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-29 18:05:42 +00:00
Marc SchierandCopilot	cef83550cb	OPC UA Vision: a vision-guided bin-picking demo an LLM agent can see and drive (#4235) ... # Description Implements **OPC UA — Vision** and uses it to build the demo the companion specifications were drafted for: an eye-in-hand camera watches a bin of parts, a language model *looks at the frame*, decides which part matches the instruction, and commands the robot through Robot Intent — with the OpenUSD viewer showing the arm move and the bin empty out. > **Draft, and stacked.** [#4195](https://github.com/OPCFoundation/UA-.NETStandard/pull/4195) has merged and master is merged in, so what is left on top of `master` is the Vision work plus the unmerged `marcschier/ai-model-management` branch, which the demo needs for the model/dataset/deployment provenance. Review the Vision commits; the AI Model Management work lands with its own PR. ## What is here **The specification, implemented.** `Opc.Ua.Vision` is generated from the spec's own `Opc.Ua.Vision.NodeSet2.xml`, vendored byte-identically (SHA-256 `79C592C6…`, verified against a fresh download and stored LF-preserved like the Robot Intent NodeSet). `Opc.Ua.Vision.Server` builds the §4.2 discovery object, sensors, the coordinate-frame tree, calibrations, media endpoints, inference pipelines and the §9 feedback methods, with rendering and inference behind injectable providers. `Opc.Ua.Vision.Client` is built on the generated ObjectType proxies and composes transforms across the frame tree, which is what turns a detection in camera coordinates into a pose a robot can act on. **Two perception paths behind one contract**, which is exactly what `InferenceLocationEnum` is for. `OnServer` is a deterministic detector derived from the stage's ground truth — no model, no network, no GPU, so CI can exercise the whole loop. `EdgeOffServer` is the agent: it sees the frame over MCP and calls `SubmitDetections`, and the Server publishes results it did not compute. A client reads `DetectionResultType` identically either way. **The agent's eyes.** `tools/Opc.Ua.Mcp.Vision` adds 22 `vision_*` tools, which with the four shared connection tools makes the `vision` profile 26; `vision_get_frame` returns the camera image as an MCP `ImageContentBlock`, so the model looks at pixels rather than reading a description of them. Tool profiles now compose — `--profile vision,robotics` yields 62 tools — so one agent can both see and act without dragging in the full 136-tool catalogue. **The cell.** `samples/Robotics/BinPickingCell` follows the spec's own Robotics-Vision Addendum: the `world → robot_base → flange → gripper_tcp` frame tree with `camera_eih` on the flange, the `HandEye` extrinsics and the `Intrinsics612x512` intrinsics — the addendum's 2448×2048 calibration scaled to the 4×4-binned grid the camera actually delivers. `samples/Robotics/BinPickingClient` hosts the composed MCP catalogue and optionally the viewer. **Documentation.** `docs/Vision.md` (~800 lines) covers the model, hosting, the build context, every topology builder, both perception paths, feedback, the MCP profile and the limitations; linked from `docs/README.md`, with the profile table in `docs/McpServer.md` and READMEs for both samples. ## Evidence, not assertions Each of these was measured on a running system rather than assumed: | Claim | Evidence | |---|---| | The camera sees the bin | 612×512 frame with the bin and all five parts in view — red `(212,47,47)`, green `(61,193,71)`, blue `(54,85,218)`, yellow `(215,196,58)`, orange `(209,119,55)` | | An agent can act on what it sees | Over MCP, `vision_get_frame` returns a 612×512 PNG that decodes from base64 to 1,254,051 bytes, the descriptor beside the detections says 612×512, and all five `BoundingBox2D` centres land **inside** the frame on the pixel colour their own detection claims | | The geometry is right | A detection composed `camera_eih → world` lands on the authored position, residual **0.0000 m** — on both the on-server and off-server paths | | The detector tracks the world | Placing a part drops the next scan from 5 detections to 4, with that part absent | | The robot obeys the agent | Scripted run took command authority and completed a pick and a place, both `Succeeded` | | Tool catalogues are honest | Counted from a running server: vision 26, robotics 40, `vision,robotics` 62, full 136, **zero duplicates** | | Every node is browsable by NodeId | Eight representative NodeIds — both folders, a pipeline, a frame, the sensor, its Calibrations folder, the HandEye calibration, the Media folder — all `Good` | That tool-count check matters because the profile tests collapse names into a `HashSet` and *cannot see* a duplicate registration — so the new tests count registrations, not names. ## Things that were wrong, and are not now Writing the tests found three defects of one kind — code that answered instead of refusing. A zero-norm quaternion was silently rewritten to identity, so a caller with an all-zero orientation got a confident wrong grasp pose. `SubmitCorrection` rewrote a null `ResultId` to empty and called the sink anyway. A bad `StageIdentifier` **killed the host process** inside native `UsdStage.Open`, via an exception .NET cannot catch. All three now refuse loudly and are pinned by tests. **Nodes that existed but could not be reached.** The node manager indexes the Vision root before configurators run, so everything the fluent builder grafted on afterwards — both folders, the pipelines with their Results and Feedback, the frames, the sensors with their optics, calibrations and media endpoints — was never added to the index. Browsing *forward from a parent* worked, because that walks `NodeState.Children` in memory; browsing any of those nodes *by its own NodeId* returned `BadNodeIdUnknown`, which is how an ordinary client and the MCP discovery tools navigate. Registration is now deferred and flushed after each configurator. The first fix covered only the DI path and left the documented non-DI fallback broken in exactly the same way, so `ConfigureVisionAsync` now exists and registers what it builds. That defect survived a green suite because every test browsed children. The new tests assert by NodeId, and were checked against the broken code — with the flush removed, they fail. **Every Method was uncallable.** The Server passed 426 unit tests while not one of its Methods could be invoked over a real session — `RunInference` returned `Bad_TooManyArguments`, and so did every other Method that takes arguments, which is all of them but `StartContinuous` and `Stop`. Three independent faults, each sufficient on its own: the Method nodes were never created (they are Optional children, and the dispatcher guards every attachment with a null check, so it silently attached nothing); nothing declared the arguments, so the stack concluded none were expected; and `MethodDeclarationId` pointed at a synthesised NodeId rather than the NodeSet declaration a client calls with, so the lookup missed the instance Method entirely. The builder now creates the Methods, the `Results` folder and the `Feedback` object when a provider or sink is configured, declares every signature, and names the right declaration. Unit tests could not have caught this: they invoke the handler delegates directly and never go through `MethodState.Call`. It took an end-to-end test over a real session. Relatedly, children created by the generated `CreateOrReplace` helpers carry no `ReferenceTypeId`, so nothing could reach them — fixed once in the build context rather than at the several dozen call sites. **And one where I was wrong.** Earlier in this branch I reversed my own agents' decision and made `SubmitDetections` accept an empty detection set, reasoning that "I looked and the bin is empty" is a correct observation and refusing it forces a correct agent to invent a detection. The reasoning is sound; it is also an argument *against* the specification rather than an implementation of it. §9.5 is normative and explicit — its StatusCode table lists `Bad_InvalidArgument` for "`Detections` empty; or `SubmitCorrection` supplies both or neither corrected array". Worse, **none of §9.5's argument rules were enforced anywhere** — the dispatcher forwarded every submission straight to the feedback sink, so conformance depended on whichever sink a host installed. They are Server obligations, so they now live in the dispatcher, ahead of the sink, proven by strict mocks that would throw if the sink were consulted. The usability problem is real and does not go away by conforming, so it is raised where it can be fixed: [opcua-drafts#70](https://github.com/marcschier/opcua-drafts/issues/70). ## Tracking the updated drafts The Vision, Robot Intent and AI Model Management NodeSets on `opcua-drafts` `main` have moved, partly because of feedback this branch raised. All three are re-vendored byte-identically — verified by comparing git blob hashes against the upstream files rather than by eye — and the implementations follow. **Two statements that were previously inexpressible now have a wire representation.** `SubmitDetections` takes `SceneIsEmpty` and `SubmitCorrection` takes `RetractAll`, so an agent can report "I examined this frame and there is nothing in it" — the terminating condition of the bin-picking task — and can retract a false positive by correcting a result down to nothing. Both are checked in both directions, because the flag is exactly what separates a deliberate empty observation from a lost payload: an empty array without the flag is refused, and the flag with an array attached is refused as two contradictory claims about one frame. Implemented in the dispatcher, the client, the MCP tools the agent drives, and the sample cell. Also absorbed: `LampType` and `LightingMode` became enums (§66), `ProfileName` became `DefaultProfileName` on the endpoint (distinct from the `ProfileName` a caller passes to `GetStreamEndpoint`, which keeps its name), AI `Invoke`/`InvokeAsync` gained `PayloadUri` under the same exactly-one rule, and AI `ListModels` gained a `ContinuationPoint` — because `MaxResults` alone puts everything past the bound permanently out of reach. **OpenUSD is on `0.9.0-alpha`**, verified on the running cell rather than by restore alone: D3D12 backend, 612×512 frame, the bin and all five parts in view and detected — the same result `0.8.0-alpha` produced. The one local workaround that survives is `VisionMethodArguments`, and its reason has changed rather than gone away: the builder materialises these Methods as Optional children through `CreateOrReplace`, which constructs the state object directly and never runs the generated factory that now carries the signatures. Checked rather than assumed — removing the declarations fails five of the eight `VisionMethodSurfaceTests`. ## Review feedback Three threads, all addressed in `0fe2803a3`: - **The AI Model Management work is now a library family, not a sample.** `src/Opc.Ua.AI` (model), `.Inference` (`IInferenceBackend` and backends), `.Server` (node manager) and `.Client` (discovery, reads, calls, artefact transfer), in the same shape as the Robotics and Vision families; the samples become `samples/AI/ModelManagementServer` and `ModelManagementClient`. The client library needed real work rather than relocation: its browsing surface was `private` inside the sample's scenario runner, so moving the file alone would have produced a library nobody could call. - **No cloud-vendor SDK is referenced anywhere.** Inference goes through `Microsoft.Extensions.AI` — an `IChatClient` is implemented by hosted services and on-device runtimes alike, which is exactly the property clause 8.1 asserts. `Azure.Identity` is gone: workload identity is not an Azure feature, every platform projects a token into a file and each SDK reads that file behind its own API, so the resolver reads it directly. The comment claiming `Microsoft.Extensions.AI` would force a `System.Text.Json` bump was stale — it pins 10.0.10, which this repository already centralises on. - **`BinPickingCell` and `BinPickingClient` moved into `samples/Robotics/`**, which is where they belonged: the cell was already linking the arm, its kinematics and both USD assets out of `IntentEnabledRobot`. ## Upstream Work that belonged elsewhere went elsewhere rather than being worked around locally: - **`openusd-dotnet` [#13](https://github.com/marcschier/openusd-dotnet/pull/13)** — merged and shipped in `0.8.0-alpha`. `SilkFrameCapture.Capture` rendered only the first capture of a session and silently returned a blank frame for every one after it. Also filed [#14](https://github.com/marcschier/openusd-dotnet/issues/14) (no camera-by-prim API) and [#15](https://github.com/marcschier/openusd-dotnet/issues/15) (skipped tests reported as passed — which fooled me for several runs). Both are fixed; this branch takes `0.8.0-alpha` and deletes two workarounds, including 143 lines of hand-rolled projection maths. - **`opcua-drafts` [#66](https://github.com/marcschier/opcua-drafts/issues/66)–[#69](https://github.com/marcschier/opcua-drafts/issues/69)** — a close read of the spec produced 22 candidate ambiguities; 12 were discarded as misreadings with clause citations and the remaining 10 filed. One is concrete: five members exist in the NodeId CSV that no clause documents. - **`opcua-drafts` [#70](https://github.com/marcschier/opcua-drafts/issues/70)** — §9.5 could not express an empty observation or a false-positive retraction. **Adopted**, with the explicit-flag resolution argued for: `SubmitDetections` gained `SceneIsEmpty`, `SubmitCorrection` gained `RetractAll`, and the corrected-array rule relaxed from *exactly one* to *at most one*. - **`opcua-drafts` [#71](https://github.com/marcschier/opcua-drafts/issues/71)** — the Vision NodeSet browse-named all 13 `InputArguments` / `OutputArguments` Properties in the Vision namespace rather than the base namespace, so a stack could not find them and every Method in the specification was uncallable. **Fixed**; this branch now tracks the corrected artefact. ## The defect underneath it all The end-to-end suite was removed earlier in this branch because nine of its fourteen tests failed on one symptom: a client could not open a pipeline's `Feedback` object or enumerate its `Results`, even though browsing the same parent listed both. Restoring it meant explaining that, and the explanation was not in Vision. `RelativePathElement` initialised **`IsInverse` to `true`**. A caller that sets a `ReferenceTypeId` and a `TargetName` — which reads as *"the child reached by this Reference"* — was therefore asking the Server for the **inverse** Reference, and got `Bad_NoMatch` with nothing in the request to suggest why. `ObjectTypeClient.ResolveChildNodeIdAsync` is such a caller, and it is what **every source-generated Optional-child accessor in the stack** is built on — the accessor whose own documentation cites `AlarmConditionTypeClient.GetShelvingStateAsync` as an example. Vision is merely where it surfaced, because Vision puts two Optional children on the path an off-Server agent must take. The sibling helper in `StateMachineTypeClientExtensions` already set `IsInverse = false` by hand, which is the shape of a trap rather than a default. The default is now forward, `ResolveChildNodeIdAsync` says so explicitly, and the existing test that had recorded `true` as *expected* now pins `false` with the reasoning. Of the forty `RelativePathElement` construction sites in the tree, that accessor was **the only one relying on the default at all** — every deliberate caller already set it. Two further defects were found underneath it, both real and both fixed: - The generated `CreateOrReplace` helpers construct a child state object directly and leave **`TypeDefinitionId`** unset as well as `ReferenceTypeId`. A child referenced by nothing cannot be browsed; one with no type definition is a malformed Object that a client filtering by type silently skips — which is exactly what `EnumerateResultsAsync` does. Normalisation moved to `VisionNodeManager`, so it also covers the case the builder cannot see: a result published **at runtime** by an inference provider, long after the address space was built. That path produced results a client could list but not read. - `Opc.Ua.AI` shipped without an `AssemblyInfo`, so the solution built with six `CA1014` warnings. I had initially "fixed" this in the Vision client with Browse-based fallbacks, on the strength of a probe that appeared to show `TranslateBrowsePathsToNodeIds` failing even for `Server → ServerStatus`. That probe was wrong: it used an unqualified `ReferenceTypeIds`, which inside a `Opc.Ua.Vision.*` namespace binds to *Vision's* `ReferenceTypeIds` and yields a nonsense NodeId. Chasing that to the end is what turned up the real cause. **The fallbacks are gone** — they would have hidden a stack-wide bug behind a Vision-shaped workaround, and one of them matched on browse name while ignoring the namespace index. `tests/Opc.Ua.Vision.Intent.Tests` is restored and back in `UA.slnx`, passing **14/14** on its own merit rather than through a client-side workaround. A registration test now pins the pipeline's `Feedback` and `Results` children, which nothing covered before. ## Running it, and what that found The demo's premise is that a language model **looks at the bin and reasons about what it sees**. Running it end to end showed it could not, for reasons no test covered, because every one of them lived between components. **The sample did not compile.** Two `CS8604` nullable errors with `TreatWarningsAsErrors`, so neither the cell server nor the demo could be started at all. Worth recording that my first explanation was wrong: CI *does* build this project - the Linux leg's `net10.0` pass compiles it, log-confirmed - so this is not a coverage gap in the workflow, and no workflow change was made. **`Pick` and `Place` never moved the arm.** They were a delay plus a gripper action, so the operations reported `Succeeded` while nothing travelled. They now solve IK once and interpolate in joint space. **And the parts never moved**: the arm swung but the bin stayed full, because nothing tied the held object to the world state. The connector now creates 11 monitored items where it created 6, and a scripted run delivers `SeqNo=151` across 10 messages where it delivered `SeqNo=3` across 1. **The frame was smuggled through a string field.** The media provider put a base64 *data URI* of the PNG into `VisionImageReferenceDataType.Uri` - a field meant to be a *reference*, while the bytes were already returned in the inline `ByteString`. Every response therefore shipped the image twice, and a 1,311,400-byte PNG became a 1,748,536-character string against a `MaxStringLength` of 65,535: **26.7x over**. That is why raising `MaxByteStringLength`, `MaxArrayLength` and `MaxMessageSize` never helped - none of them govern a `String`. Separately the MCP tool assigned raw bytes where `ImageContentBlock` requires base64, so the wire carried `"data":"�PNG..."` - every non-UTF-8 byte replaced by U+FFFD, destroying the image and inflating the response to 6.8 MB. **The camera was aimed at nothing.** The arm's home pose, the flange scan pose the Vision model declared and the joint angles in the USD stage were three independent claims about where the camera was, and they disagreed - the declared flange orientation pointed the view 86 degrees away from the projection camera. Solving them as one thing exposed two further constraints that are easy to miss: the camera has to sit off the tool axis or it photographs its own gripper, and the IK branch has to be elbow-back or a link parks under the camera and fills the frame with the arm's own upper arm. A third was a regression this branch introduced and `Opc.Ua.Robotics.Tests` caught - aiming a straight-down camera from a point on the base's own X-Z plane puts the wrist on the J4/J6 singularity, so every motion away from home fails to solve. A 15-degree camera roll, free because the camera still looks straight down, gets 25 degrees clear, which is the margin the poses before this branch had. **The frame and the detections were different images.** The sensor declared 2448x2048, the clip endpoint 1280x1024, and the renderer produced 640x512 - not even the same aspect ratio. The ground-truth detector projects through the declared intrinsics, so its boxes lived in 2448x2048 space while an agent was handed a 640x512 picture: *"pick the red cube you can see"* arrived with coordinates that pointed off the image. Everything now agrees at 612x512, an exact 4x4 bin of the native sensor with the intrinsics scaled to match; the native size stays in the model and serial number, where it describes the hardware rather than the image. **`LatestClip` was created and never written**, so it reported `Bad_NoDataAvailable` for the life of the Server and a consumer following the model - read the published frame, call the method only if there is none - never got a frame. The dispatcher now publishes a clip it has just encoded, for every Vision server rather than this sample alone. One genuine API gap turned up on the way: `OpcUaServerOptions` exposes `MaxByteStringLength`, `MaxArrayLength` and `MaxMessageSize` and **no `MaxStringLength`**, so a Server that legitimately needs a long string could not raise the limit through the hosting API at all. Added, with tests. It is not the fix for anything above - that was a design bug - but the missing knob was real. ## Not finished - **Two projects sit below the repository's 80% line coverage.** `Opc.Ua.Vision.OpenUsd` is at 71.1% — what is left only executes with a real OpenUSD stage and plugin tree, which is not on every build agent. `Opc.Ua.AI.Inference` is now at **86.7%**, up from 12.9%: the code this branch *wrote* is covered by 25 tests, and `RestChatCompletionsBackend` — previously the whole of the shortfall — is covered by 18 more against a stubbed `HttpMessageHandler`, exercising success, refusal, throttling, retry-after, timeout and cancellation without a network. Everything else clears the bar — `Opc.Ua.Vision.Server` 92.1%, `Opc.Ua.AI` 91.4%, `.Vision.Client` 88.1%, `Opc.Ua.Vision` 85.7%, `Opc.Ua.AI.Server` 80.1%, all measured with coverlet. The CI coverage gate reports itself as advisory and non-blocking. - **`SamplesCollected` is not counted here, and `docs/Vision.md` now says so.** Section 9.4 requires a Server to count a negative example (`SceneIsEmpty` / `RetractAll` carrying a `GroundTruthLabel`) exactly as it counts one carrying geometry. `VisionNodeManager` does not, and structurally cannot: `SamplesCollected` is a property of `LearningJobType`, which the *AI Model Management* companion defines, and Vision reaches the job through a `NodeId` value rather than a Reference **precisely so this model takes no dependency on the model that defines it**. What the Server does guarantee is that the negative example survives the hop intact — both flags are carried verbatim to `IVisionFeedbackSink` — so a host that binds a learning job has everything it needs to satisfy the clause on the counter it owns. Stated in the limitations rather than left implied. ## Related Issues Was stacked on #4195, now merged. No tracking issue yet for the Vision implementation itself — happy to open one if you would like the design recorded as an ADR before this goes further. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. — *Types 8562, Server 4072, Client 2123, InformationModel 1063, Robotics 540, Tools 466, Vision 442, AI 74 and the restored Vision+Intent end-to-end suite 14/14, all green on net10.0. Only `.OpenUsd` (71.1%) is below the 80% bar — see Not finished.* - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. — *`dotnet build UA.slnx -c Release`: 0 errors, 0 warnings.* - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. — *net48: Types 8555, Client 2126, Vision 385 green. Two net48 Server tests fail (`CreateUserManagementSeamBindsTheModelToTheServerAsync`, and `DurableDataValueQueueVerifyReferenceBatchingAsync` only under parallel load); both were confirmed **pre-existing** by re-running them with the `IsInverse` change reverted, which fails identically.* - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. — *The net48 build legs failed because the restored end-to-end project used `ValueTask.FromResult` / `string.Create`, which do not exist on .NET Framework; fixed in `fb57522da` and verified building on every TFM. `DisposeDrainsHeldConnectionCallbackBeforeListenerDisposal` failed once on the Ubuntu Client leg — a 13 ms timing-sensitive reverse-connect disposal test that passes locally 2123/2123 and touches no browse paths.* - [ ] I have addressed **all** PR feedback received. ## Typed Vision-guided Robotics MCP workflows The branch now also makes the demo workflow first-class in the Full MCP profile: - replaces stringified Robotics intent and mission inputs with typed request schemas and exact name-or-NodeId selectors; - adds bounded operation/mission paging, stable step-to-operation identity, retained mission discovery, and `robotics_wait_mission`; - makes `vision_run_inference` return bounded typed detection, inspection, or segmentation summaries in one call; - adds same-session `robotics_vision_pick` to select a detection and submit Pick or Pick/Place work without implicitly taking authority, retrying, cancelling, or waiting; and - starts Publish workers for classic subscriptions so an independently launched OpenUSD viewer receives live robot and part updates. The migration guide documents the intentionally breaking MCP request-schema changes. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b Copilot-Session: 99d1ab42-f83a-41ab-bf3e-85bf81fadace Copilot-Session: bcbc8ac9-5abd-42a8-8bac-e10533790e2e	2026-08-29 19:34:15 +02:00
4 people	70c8ce4649	Allow passing an existing config XML to the client DI AddClient (#4328) ... # Description > **Stacked PR**: based on `romanett/server-di-config-xml-e30c4f` (#4325) so the diff shows only the client changes. Once #4325 merges this will be rebased onto `master`. Gives the **client** DI surface the same migration path for existing configuration XML files that #4325 gave the server: applications that already own a classic `MyClient.Config.xml` can pass it to `AddClient` and keep **all** of their settings. New API surface: - `AddClient(string configurationFile, Action<OpcUaClientOptions>? configure = null)` loads the `ApplicationConfiguration` from the file; the optional callback configures the remaining client options (session endpoint, identity, reverse connect, ...). - `AddClient(Stream configurationStream, ...)` for documents that are not files on disk, e.g. embedded resources. The stream is read once and disposed after loading. - `OpcUaClientOptions.ConfigurationFile` / `ConfigurationStream` back the overloads, so the options-callback route works too and the file path can be bound from configuration (`OpcUa:Client:ConfigurationFile`). - `OpcUaClientOptions.ConfigureLoadedConfiguration` runs after the document is loaded and validated but before the application-instance certificate check, for code-side overrides of individual file settings. Mechanism and behavior: - A new internal `ClientSuppliedConfigurationProvider` implements `IOpcUaApplicationConfigurationProvider` and plugs into the client's existing configuration-provider seam. The document loads **lazily on first use** — the first session connect, reverse-connect startup, or an explicit `GetAsync` — through `ApplicationInstance.LoadApplicationConfigurationAsync`, so every setting (security configuration, certificate stores, transport quotas, client configuration) applies exactly as on the classic path, including localhost fix-up. - The load **ensures the application-instance certificate** (mirroring the shared `ConfigureApplication(...)` provider), which the pre-existing manual workaround — pre-loading and assigning `OpcUaClientOptions.Configuration` — never received. DI-registered `ICertificateManager` / `ICertificatePasswordProvider` are honored. Single-flight loading; `IAsyncDisposable` like the shared provider. - The supplied document is authoritative and takes precedence over a shared application registered with `ConfigureApplication(...)`. Ambiguous combinations are rejected eagerly at `AddClient` with clear `InvalidOperationException`s: file + stream, document + explicit `Configuration`, document + application identity properties, and a white-space path arriving through configuration binding. The start-time `IValidateOptions` validator accepts a supplied document as a configuration source. - `ConnectManagedSessionAsync` publishes the lazily loaded configuration onto the resolved options after `GetAsync`, so the channel manager and session builder see it exactly as on the other configuration paths; reverse connect consumes the provider through its existing async-startup path unchanged. Coverage: new `ClientConfigurationFileTests` fixture (8 tests) — loading from file and stream with assertions on the document settings, the override callback, the certificate bootstrap, provider caching and lazy semantics, precedence over `ConfigureApplication`, load failure for a missing file, argument validation, ambiguous-combination rejection, configuration-section binding, and validator acceptance. All 323 existing ClientBuilder tests and all 180 server hosting tests (including the combined client+server fixtures) pass; `Opc.Ua.Client` builds warning-free on net472, net8.0, and net10.0 (AOT analyzers). Docs: new "Client: using an existing configuration XML file" section in `docs/DependencyInjection.md` with cross-links from the server section, an updated pointer in the 2.0 migration guide, and a NuGet README mention. ## Related Issues - Fixes #4327 - Stacked on #4325 (server-side counterpart) ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-29 13:33:40 +00:00
4 people	e61ad4b306	Fully support sub-state machines in the fluent state-machine builder (#4322) ... # Description Part 16 §B.3 places `HasSubStateMachine` on the parent **state** node, but the fluent builder attached it to the state-machine **root** because `FluentFiniteStateMachineState` never materialized state nodes at all. Investigating #3821 showed the gap was wider than the reference placement: 1. **No state nodes existed.** The NodeId written into `CurrentState/Id` — `new NodeId(stateId, ElementNamespaceIndex)` — was **dangling**: no node with that id was in the address space. 2. **`AvailableStates` was never populated**, so `GetAvailableStatesAsync` returned empty against a fluent-built server. 3. **The whole client discovery chain was therefore dead.** `ObserveEffectiveStateAsync` enumerates `AvailableStates` and browses `HasSubStateMachine` from each state node — it found nothing. The client side was already spec-correct; only the server deviated. 4. **`HasSubStateMachine` is non-hierarchical** (a subtype of `NonHierarchicalReferences`, `i=32`). Using it as the sub-SM's `BaseInstanceState.ReferenceTypeId` left the sub-SM unreachable from a hierarchical browse of the machine. ## What changed **Element nodes are materialized.** Definition-mode machines now create one `StateType` node per declared state and one `TransitionType` node per declared transition — each a `HasComponent` child carrying `StateNumber` / `TransitionNumber`, with the §B.4 `FromState` / `ToState` / `HasEffect` references and `HasCause` added by `WithCause`. `AvailableStates`, `AvailableTransitions` and `LastTransition` are populated, so `CurrentState/Id` and `LastTransition/Id` resolve to real, browsable nodes. NodeIds are per-instance, so several machines built from one definition coexist. **`HasSubStateMachine` moved to the state node**, with the inverse `SubStateMachineOf`, and the sub-SM is attached as a `HasComponent` child (fixing 4). **Core gains element-id mapping hooks.** `GetCurrentStateId` / `UpdateStateVariable` / `UpdateTransitionVariable` hard-coded the numeric form and were non-virtual, so a subclass could not redirect them. They now route through `public virtual` `GetStateNodeId` / `GetStateId` / `GetTransitionNodeId` / `GetTransitionId`, whose defaults reproduce the previous `(elementId, ElementNamespaceIndex)` convention **exactly** — every stack-shipped and generator-emitted machine is unaffected, and there is a regression test asserting that. **Generated machines report their model namespace.** The generator now emits an `ElementNamespaceUri` override on `FiniteStateMachineType` subtypes, taken from the nearest type that actually *declares* state children — a subtype that only adds behaviour inherits its elements, and their namespace, from its base. **Ordering and validation fixes.** `WithSubStateMachine` and `WithInitialState` are now order-independent (previously, attaching a sub-SM before setting the initial state left the child suspended until the parent left and re-entered the state — silently). `WithSubStateMachine` validates `parentStateId`, and the builder rejects duplicate element browse names, since element NodeIds are now derived from them. ## A defect this surfaced Code that assembles nodes with the generated `CreateInstanceOf<Type>` factories never calls `NodeState.CreateAsPredefinedNode`, so `OnBeforeCreate` / `OnAfterCreate` are skipped for the entire subtree — and `OnAfterCreate` is the only place a state machine resolves its element namespace. It is also where `ConditionState` binds its `Enable` / `Disable` / `AddComment` handlers and `AlarmConditionState` binds the shelving methods, so the impact is broader than state machines. This PR fixes only the two call sites it depends on — DI's `SoftwareUpdateFacetWiring` and Robotics' `RoboticsOperationBuilder`. **The repository-wide audit is deliberately left out of scope and tracked in #4320** (24 files call these factories; none completed the lifecycle). The Robotics regression had no test coverage at all, so an assertion on `CurrentState/Id` was added and verified to fail without the fix. ## Breaking changes - Sub-state machines are **no longer browsable from the state-machine root**. Callers passing a machine's `ObjectId` to `GetSubStateMachineAsync` must pass a state NodeId instead — from `GetAvailableStatesAsync` or a snapshot's `CurrentStateId`. - `CurrentState/Id` for fluent machines changes from the numeric form to the materialized node's id. Code reading a numeric id out of it should call `FiniteStateMachineState.GetStateId` rather than parsing the identifier. - Generated machines qualify `CurrentState/Id` / `LastTransition/Id` with their companion-spec namespace instead of the OPC UA one. Documented in `docs/WhatsNewIn2.0.md` and `docs/StateMachines.md`. ## Notes for reviewers - Adding `virtual` to the `FiniteStateMachineState` members is a binary (not source) breaking change — assumed acceptable on the 2.0 line, but worth confirming against the compatibility policy. - Address-space cost is one node plus one property per declared state and transition, per machine instance; called out in the docs for servers instantiating many machines. - `AvailableTransitions` is modelled as a `HasComponent` `BaseDataVariable` (matching the standard NodeSet and what the client's browse path expects), not a Property. ## Related Issues - Fixes #3821 - Surfaced #4320 (node create lifecycle audit — not addressed here) ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ### Test runs (net10.0) | Suite | Result | |---|---| | `Opc.Ua.Server.Tests` (full) | 4734 passed, 5 skipped | | `Opc.Ua.SourceGeneration.Core.Tests` | 3780 passed | | `Opc.Ua.SourceGeneration.Tests` | 132 passed | | `Opc.Ua.SourceGeneration.Stack.Tests` | 94 passed | | `Opc.Ua.Types.Tests` | 8763 passed | | `Opc.Ua.Robotics.Tests` | 540 passed | | `Opc.Ua.Robotics.Intent.Tests` | 22 passed | | `Opc.Ua.Di.Tests` | 424 passed | | `Opc.Ua.ISA95.Tests` | 136 passed | | `Opc.Ua.Aas.Tests` | 670 passed | | `Opc.Ua.Positioning.Tests` | 69 passed | | `Opc.Ua.Client.Tests` (StateMachines) | 36 passed | Only net10.0 was run locally; the .NET Framework leg is left to CI, hence the unchecked box above. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-29 13:33:16 +00:00
romanettandClaude Fable 5	f4e93eab93	Fix public IsNodeInView/IsNodeInViewAsync overloads always returning false (#4333) ... # Description The public `IsNodeInView(OperationContext, NodeId, object)` in `CustomNodeManager2` and `IsNodeInViewAsync(OperationContext, NodeId, object, CancellationToken)` in `AsyncCustomNodeManager` unwrap the `NodeHandle` and forward `handle.Node` together with the **unchanged** `OperationContext`. Since there is no `NodeState`-taking overload with an `OperationContext`, overload resolution binds the call back to the same public `(object)` overload; the recursive call's `is not NodeHandle` guard then returns false. As a result both entry points always reported "not in view" for a validated handle, regardless of the view. The sync variant has behaved this way since the initial commit; the async port copied the same pattern. The defect is latent in the SDK today: nothing calls `INodeManager2.IsNodeInView` / `IsNodeInViewAsync` except the sync/async adapters forwarding to each other, and Browse view filtering flows through the protected virtuals directly. External code calling the public API (e.g. node managers ported from the samples) was affected. Changes: - Both public overloads now copy the operation context (`SystemContext.Copy(context)`) and delegate to the overridable protected `IsNodeInView(ServerSystemContext, NodeId, NodeState)` helper — the same virtual Browse filtering uses and sub-classes override. Doc remarks name the virtual to override. `IsNodeInViewAsync` drops its now-unneeded `async` keyword (signature unchanged). - New regression test `IsNodeInViewAsync_HandleWithNodeUsesOverridableIsNodeInViewAsync` runs against the native async manager (both monitored-item manager configurations) and against `CustomNodeManager2` behind `AsyncNodeManagerAdapter`. It covers: known view → true, unknown view → false, placeholder handle without node → false, the sync entry point via `SyncNodeManager`, and — via an override hook on the testable managers — that the public entry points genuinely route through the overridable protected virtual. The test was verified to fail against the unfixed code. - The characterization test that locked in the broken sync behaviour (`IsNodeInView_ValidatedHandleReturnsFalseFromPublicOverloadRecursion`, flagged "not fixed here") is replaced by `IsNodeInView_ValidatedHandleDelegatesToProtectedOverload`, asserting the fixed behaviour. Verified locally: all `IsNodeInView*` tests and the full node-manager suites (484 tests) pass on net8.0; `Opc.Ua.Server` builds warning-free on net472. ## Related Issues Found while migrating the Views sample, whose view filtering uses the protected sync virtuals and is unaffected. - OPCFoundation/UA-.NETStandard-Samples#758 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-29 13:59:50 +02:00
4 people	33ed3028e6	Fix macOS test host crash caused by indefinite keepAlive worker await in session teardown (#4319) ... `test-macOS-latest-History` CI job crashes after all 293 tests pass. All tests complete successfully but the test host exits with code 1 ("Test host process crashed") ~35 seconds into teardown. **Root cause chain:** - `Session.StopKeepAliveTimerAsync` unconditionally `await`s `keepAliveWorker`, which is blocked in `ReadAsync`. On macOS under CI load, socket reads do not honor `CancellationToken`, causing session teardown to hang indefinitely. - After 15 s, `TestFixture.OneTimeTearDown`'s watchdog fires, sets `s_skipRemainingTeardowns = true`, and bails out—without calling `ServerFixture.StopAsync()`, leaving certificate managers un-disposed. - Certificate leak assertions fire in `GlobalTeardown`, triggering `ArmProcessExitWatchdog` → `Environment.Exit(0)` on a background thread severs the VSTest pipe → "Test host process crashed", exit code 1. ## Changes **`src/Opc.Ua.Client/Session/Session.cs`** - `StopKeepAliveTimerAsync`: replace unconditional `await keepAliveWorker` with a 5-second bounded `Task.WhenAny`. If the worker doesn't stop in time, log a warning and abandon it. Well-behaved transports cancel immediately; the bound guards against unresponsive macOS sockets. - Added `s_keepAliveStopTimeout` static field (5 s) and `KeepAliveWorkerDidNotStopWithinTimeout` source-generated log message (`Session+69`). ```csharp // Before await keepAliveWorker.ConfigureAwait(false); // After var completed = await Task.WhenAny(keepAliveWorker, Task.Delay(s_keepAliveStopTimeout)).ConfigureAwait(false); if (completed != keepAliveWorker) { SessionLog.KeepAliveWorkerDidNotStopWithinTimeout(m_logger, s_keepAliveStopTimeout); } ``` **`tests/Opc.Ua.Client.TestFramework/TestFixture.cs`** - `OneTimeTearDown`: both the early-skip and post-session-close skip paths now call `ServerFixture.StopAsync()` before nulling the reference. `StopAsync` fast-paths when `s_skipRemainingTeardowns` is set—disposes certificate managers without blocking on server shutdown—breaking the leak→crash cascade. ## Related Issues ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: romanett <romanett98@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-08-29 10:34:55 +00:00
romanettandClaude Fable 5	b9657e5f0d	Allow passing an existing config XML to the hosted server DI AddServer (#4325) ... # Description Extends the hosted-server dependency-injection surface so applications that already own a classic `ApplicationConfiguration` XML file (`*.Config.xml`) have a direct migration path to DI with **all** of their existing settings applied. New API surface: - `AddServer(string configurationFile, Action<ApplicationConfiguration>? configure = null)` and the `AddServer<TServer>` variant load the configuration from the file instead of building it from `OpcUaServerOptions`. - `AddServer(Stream configurationStream, ...)` (plus generic variant) for documents that are not files on disk, e.g. embedded resources. The stream is read once and disposed by the hosted service during startup. - `OpcUaServerOptions.ConfigurationFile` / `ConfigurationStream` back the overloads, so the options-callback route works too and the file path can be bound from configuration (`OpcUa:Server:ConfigurationFile`). - `OpcUaServerOptions.ConfigureLoadedConfiguration` runs after the document is loaded and validated but before certificates are checked, so individual settings can be overridden from code without editing the file. Behavior: - The document is loaded through `ApplicationInstance.LoadApplicationConfigurationAsync`, so every setting (base addresses, security policies, certificate stores, transport quotas, operation limits, user token policies) applies exactly as on the classic path, including localhost fix-up. - The supplied document is authoritative: the builder-feeding options (`ApplicationName`, `EndpointUrls`, `PkiRoot`, policy toggles, `ConfigureBuilder`, ...) are not applied on this path, and it takes precedence over a shared application registered with `ConfigureApplication(...)`. Options acting on the hosted server itself (`Identity`, `ConfigureRateLimits`) and all fluent registrations (`AddNodeManager`, `ConfigureRoles`, identity authenticators, ...) compose unchanged. - DI-registered `ICertificateManager` / `ICertificatePasswordProvider` are honored, the "listening at" log reports the loaded base addresses, and the unmatched user-token-policy startup warning is derived from the policies the document advertises. - Setting both `ConfigurationFile` and `ConfigurationStream` fails startup with a clear `InvalidOperationException`; a missing file fails startup with the original `ServiceResultException`. Coverage: new `ConfigurationFileHostingTests` fixture (12 tests) with end-to-end hosted starts from file and stream asserting the loaded settings, custom `TServer` types, the override callback, precedence over `ConfigureApplication`, warning derivation from the document, missing-file and file+stream startup failures, argument validation, the duplicate-registration guard, and configuration-section binding. Docs: new "Migrating with an existing configuration XML file" section in `docs/DependencyInjection.md` (plus first-class options table rows), a pointer in the 2.0 migration guide, and a NuGet README mention. ## Related Issues None — additive convenience overloads on the existing hosted-server dependency-injection surface; no behavior change for existing registrations. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-29 10:48:24 +02:00
romanettandClaude Fable 5	41db5aca60	Fluent: publish Configure-created nodes' references on foreign-manager nodes (Objects folder placement) (#4331) ... # Description A node created inside the fluent `Configure` hook (source-generated `[NodeManager]` partial, or the hosting `AddNodeManager(uri, build)` route) had no way to appear below the Objects folder — or below any node owned by another node manager. `AsyncCustomNodeManager.AddReverseReferencesAsync` mirrors inverse references into the `externalReferences` dictionary **before** the `Configure` partials run, and the fluent surface never sees the dictionary, so a configure-created node with an inverse `Organizes` reference to `ObjectsFolder` existed and simulated but never showed up under Objects. This implements the three coordinated pieces proposed in #4329, plus the companion namespace gap: **1. Idempotent mirroring pass** (independently worthwhile hardening) - `AddExternalReference` in `AsyncCustomNodeManager` and `CustomNodeManager2` now has if-missing semantics (linear scan of the per-source list). - `TypeTable.AddEncoding` no longer appends a duplicate encoding id; re-registering the same encoding is a succeeding no-op. (`AddRootNotifierAsync` and the node-side `AddReferenceIfMissing` branch were already idempotent.) **2. Re-run the pass after `Configure`** - New protected `FluentNodeManagerBase.CompleteConfigureAsync(externalReferences, ct)` wraps `AddReverseReferencesAsync` so hand-written managers using `CreateFluentBuilder` benefit too. - The generator-emitted `CreateAddressSpaceAsync` and the hosting `FluentNodeManager` invoke it once between the `Configure` callbacks and `Seal()`. Timing is safe: the master distributes `externalReferences` only after every manager's `CreateAddressSpaceAsync` returns. Side benefit: inverse `HasNotifier` references on configure-created event sources get root-notifier registration for free. - The hosting `FluentNodeManager`'s hand-rolled `externalReferences` workaround for its root folder is deleted — the root's inverse `Organizes` reference now flows through the shared pass (mirrored exactly once, covered by a test). **3. Discoverable fluent sugar** - `OrganizedBy(parentId)` / `UnderObjectsFolder()` on `INodeBuilder` write the inverse `Organizes` reference; with piece 2, placement is just "write the inverse reference". - A parentless `CreateInstance<TState>` on `INodeManagerBuilder` takes a constructor-style factory (`p => new BoilerState(p)`, keeping the surface reflection-free and AOT-safe), materializes the subtree from the type model via `NodeState.Create`, rebases all NodeIds through the manager's `INodeIdFactory` (`AssignInstanceNodeId`/`AssignInstanceChildNodeIds` — the same pair the generated factories use, so declaration-id children cannot collide with the type model), and registers via `AddPredefinedNodeSynchronously`. The Boiler #2 shape becomes fully fluent: ```csharp builder.CreateInstance( new QualifiedName("Boiler #2", NamespaceIndexes[1]), p => new BoilerState(p)) .Configure(n => n.UnderObjectsFolder()); ``` **Companion gap, same theme:** `[NodeManager]` grows `AdditionalNamespaceUris`, flowing through attribute discovery → binding → generator so the generated constructor reports a second (instance) namespace at construction and the generated factory advertises it in `NamespacesUris` — today `SetNamespaces` after construction is not enough because `MasterNodeManager` builds its namespace routing from what the manager reported when it was built. **Scope boundary** (as discussed in the issue): startup-time configuration only; for nodes created after startup the correct primitive remains `IMasterNodeManager.AddReferencesAsync`. **Reviewer notes** - The generated-code change was validated against the real pipeline: a forced fresh compile of `Opc.Ua.Server.Tests` (whose CoverageNodeSet managers are `[NodeManager]`-generated) emits `await CompleteConfigureAsync(externalReferences, cancellationToken)` between `Configure` and `Seal()` and compiles against the real base class. - The distribution consumer (`AddReferencesAsync`) already used `AddReferenceIfMissing`, so the whole chain is defense-in-depth idempotent. - 15 new tests: double-run idempotence, second-pass pickup of late-registered nodes, the new reference sugar, root-instance creation incl. child-id rebasing and reference remapping, hosting end-to-end Objects-folder placement, emitted-sequence and `AdditionalNamespaceUris` generator tests, and a full Roslyn-pipeline attribute test. Docs updated in `docs/NodeManagers.md` (new "Creating nodes under other managers' nodes" section, attribute docs, fluent-surface sections). - Verified locally: all touched projects build with 0 warnings; full `Opc.Ua.Server.Tests` (4,725 passed / 0 failed / 5 skipped, net9.0), `TypeTableTests` (113), `Opc.Ua.SourceGeneration.Core.Tests` (41), `Opc.Ua.SourceGeneration.Tests` (132), and the Quickstarts.Servers sample all pass. ## Related Issues - Fixes #4329 - Motivating case: OPCFoundation/UA-.NETStandard-Samples#756 / OPCFoundation/UA-.NETStandard-Samples#795 (Workshop/Boiler migration) ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-29 10:38:56 +02:00
romanettandClaude Fable 5	98c5438395	Bind monitored items to their owning node manager instead of per-call adapter wrappers (#4336) ... # Description `AsyncCustomNodeManager` passed `m_syncNodeManager.ToAsyncNodeManager()` as the owning node manager when creating and restoring monitored items. Since `SyncNodeManagerAdapter` does not implement `IAsyncNodeManager`, every call allocated a fresh `AsyncNodeManagerAdapter` around the `SyncNodeManagerAdapter` around the manager itself. The double-wrapped chain: - routed async dispatch through the sync adapter's `GetAwaiter().GetResult()` blocking (sync-over-async), defeating the purpose of the async node manager, - hid `INodeManagerMonitoredItemLifecycle` — the inner sync adapter does not implement it, so detach during monitored-item recovery degraded to `BadNotSupported` and ownership snapshots came back empty, - broke reference-identity checks against the owning node manager and gave every item a distinct `NodeManager` instance. The manager implements `IAsyncNodeManager` natively, so it now passes `this` at both call sites — the same reference its constructor already hands to the monitored item managers. The second commit applies the milder counterpart to the sync `CustomNodeManager2`, which called `this.ToAsyncNodeManager()` at five sites and thereby allocated a fresh single-wrap adapter per created monitored item. The adapter is now created once in the constructor and reused, so the monitored item managers, created and restored items, and detached-item recovery all share one `NodeManager` reference instead of relying on the `SyncNodeManager` fallbacks in `AreSameNodeManager`/`GetVisibleNodeManager`. The regression test `CreateMonitoredItemsAsyncBindsOwningNodeManagerAsync` runs in all three fixture variants (MonitoredNode, SamplingGroup, CustomNodeManager2 via adapter) and asserts the created items bind to the manager itself, share one `NodeManager` reference, and expose a working monitored-item lifecycle snapshot. Both halves were verified to fail without their respective fix. ## Related Issues - Fixes #4334 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-29 10:31:31 +02:00
3 people	a504c3e248	Fix certificate leak when rejected-store enqueue races manager disposal (#4337) ... The macOS legs of the Build and Test workflow intermittently failed the assembly-level teardown of the Sessions (also History and Client) test suites with "Certificate leak detected: 1 instance(s) created but not disposed" while every test passed (master run 33004556144: created=16777, disposed=16776; PR run 33207357247: created=17373, disposed=17372). The count held steady through the teardown's full quiescence window, so the missing disposal was permanent, not a slow in-flight drain. RejectedCertificateProcessor.EnqueueAsync takes a per-certificate AddRef for the queued WriteRequest before TryWrite. Once CertificateManager.DisposeAsync has completed the channel writer, the bounded DropOldest channel refuses TryWrite and the WriteAsync fallback throws ChannelClosedException, abandoning the request: nothing releases the references it owns, so every certificate in the chain keeps one leaked reference for the life of the process. A validation that races manager disposal — a keep-alive, reconnect or fire-and-forget channel teardown rejecting a certificate while the fixture tears down — lands in exactly this window, and the slower loaded macOS runners make that race likely. The abandoned request also left FlushRejectedAsync waiters hanging on a drain marker that never resolves, and surfaced unobserved ChannelClosedExceptions through the fire-and-forget EnqueueRejectedCertificate discard. Route both enqueue paths through a fallback that, when the write cannot commit the request to the channel, disposes the request's chain (releasing the per-certificate references), resolves its completion as "not processed" so drain waiters are unblocked, and swallows only the benign ChannelClosedException shutdown race; cancellation still propagates. The teardown leak detector needs no change: its quiescence poll was correctly reporting a genuine leak. The new regression test reproduces the CI signature deterministically against the previous code (ChannelClosedException plus "Certificate leak detected: 1 instance(s) created but not disposed (created=1, disposed=0)" in the assembly teardown) and passes with the fix. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-08-29 10:30:19 +02:00
5 people	ca9cfcc933	Inject ISecurityPolicyRegistry into Session and secure channel (#4314) ... - [x] `ChannelQuotas` + `UaSCUaBinaryChannel` carry an `ISecurityPolicyRegistry` (falling back to `SecurityPolicies.Default`) - [x] Transport channel/listener settings carry the registry and feed every `ChannelQuotas` construction site - [x] `ClientChannelManager` and `ServerBase` source the registry - [x] `Session`, `DefaultSessionFactory`, `ChannelManagerSessionFactory` and `ManagedSession` carry it - [x] Identity token handler injection points made load-bearing (`UserIdentity`, `AsTokenHandler`, default identity providers, `IdentitySelectionContext`) - [x] `SessionSecurityPolicyHelper`, `ClientIdentityProviderExtensions`, `CoreClientUtils`, `OfflineSecureChannel`, `OpcUaSecurityKeyServiceClient` - [x] Client and server DI wiring resolve `ISecurityPolicyRegistry` - [x] `IPubSubSecurityPolicyRegistry` registered in DI and threaded through the PubSub application/SKS paths - [x] Channel asymmetric crypto (padding, sign, verify, ECC classification) resolves from the negotiated policy instead of a hard-coded policy URI switch - [x] Unit tests for the registry carriers plus an integration test that negotiates a policy registered through `AddSecurityPolicy` over a real loopback handshake - [x] `docs/CryptoProvider.md` documents how the registry reaches the channel and the session - [x] Full solution build (`UA.slnx`, net10.0) and full `Opc.Ua.Core.Tests` run (4351 passed) are green - [ ] Broader integration suites (`Opc.Ua.Sessions.Tests`, `Opc.Ua.Server.Tests`, `Opc.Ua.PubSub.Tests`) were still running when time ran out - [x] Review feedback addressed: preserved original CLR signatures, routed factory channel creation through public registry-aware overloads, failed closed for unsupported asymmetric encryption, and added a provider contract for custom session factories - [x] Focused review-feedback tests pass (28 tests across Client, Core, and Core.Diagnostics) - [x] Security review: hardened SKS custom-policy admission against weak algorithms and incoherent RSA/ECC key-exchange metadata; final specialist review found no remaining vulnerabilities <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #4250 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <mschier@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: romanett <romanett98@gmail.com>	2026-08-27 05:57:17 +02:00
6 people	3f52d5b425	Wait for HTTPS listener readiness in integration tests (#4313) ... # Description The real HTTPS listener test could issue its first request before Kestrel accepted connections, causing intermittent macOS failures. - **Listener readiness** - Probe the loopback port after `OpenAsync`. - Retry connection failures every 25 ms with a 10-second deadline. - Create the test client only after the listener accepts TCP connections. ## Related Issues ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #4307 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: romanett <romanett98@gmail.com>	2026-08-26 16:39:57 -07:00
4 people	d0d048dbb3	Fix spurious BadNoCommunication reconnect loop on sessions with steady traffic (#4318) ... # Description A session with steady request traffic that then goes quiet for one `KeepAliveInterval` was flagged `BadNoCommunication` and driven `Connected -> Reconnecting` by the `ConnectionStateMachine`, even though the connection was perfectly healthy. Two parts of the keep-alive logic disagreed about what counts as proof of liveness: - `Session.RequestCompleted` treats **any** good service response as proof the server is alive and postpones the keep-alive Read by a full `KeepAliveInterval`. - The freshness timestamp `KeepAliveStopped` is judged against (`m_lastKeepAliveTimestamp`) was refreshed **only** by an actual keep-alive Read response in `OnKeepAlive` (and at timer start). So under steady traffic the keep-alive Read was perpetually deferred (the stated intent) while its freshness timestamp went arbitrarily stale. The moment traffic paused, the timer finally fired, `OnSendKeepAliveAsync` evaluated `KeepAliveStopped` against the stale timestamp and raised `OnKeepAliveError(BadNoCommunication)` **before attempting the Read**. `ManagedSession.OnInnerKeepAlive` then routed that straight to `StateMachine.TriggerReconnect()`, producing a dead-regular Connected -> Reconnecting -> Connected loop on an idle-but-healthy session. ## What changed `src/Opc.Ua.Client/Session/Session.cs`: - `RequestCompleted` now refreshes the liveness timestamp alongside the timer reset, so the same response that defers the Read also counts as proof for the `KeepAliveStopped` freshness check. Extracted into a small `UpdateLastKeepAliveTime()` helper that the three existing refresh sites (`StartKeepAliveTimerAsync`, both branches of `OnKeepAlive`) now share. - The refresh is guarded by `if (!KeepAliveStopped)`, so a **latched** keep-alive error is still only cleared by an actual keep-alive response in `OnKeepAlive`. This matters: the keep-alive Read's own `RequestCompleted` runs *before* `OnKeepAlive`, so an unconditional refresh would have made `OnKeepAlive` take the `else` branch and silently skip the recovery path (`StartPublishing`, defuncting stale publish requests) after a `BadNoCommunication`. - `m_lastKeepAliveTimestamp` is now read and written through `Interlocked`, since it is written from arbitrary response threads rather than only from the keep-alive worker. `LastKeepAliveTime` / `LastKeepAliveTimestamp` therefore now mean "last time the server proved it is alive" rather than "last keep-alive Read". That is what both existing consumers already want: `ClassicSubscriptionEngine` only logs it next to `KeepAliveStopped`, and `SessionReconnectHandler` uses it to compute the remaining session-timeout budget, which becomes more accurate. ## Test plan New `tests/Opc.Ua.Client.Tests/Session/SessionKeepAliveTests.cs` (5 tests, driven by `FakeTimeProvider`, no server needed): - `GoodResponsesKeepTheSessionAliveWhileTheKeepAliveReadIsDeferred` — the reported repro: a good response every 2 s for 20 s with a 5 s `KeepAliveInterval`. Confirmed this **fails on the unpatched code** (trips at iteration 2, i.e. 6 s in) and passes with the fix. It also asserts that once traffic stops, silence is still detected. - `KeepAliveStoppedTripsWhenNothingIsHeardFromTheServer` — negative control, the detector still works. - `BadResponsesDoNotRefreshTheKeepAliveTimestamp` — only good responses count. - `LatchedKeepAliveErrorIsNotClearedByAGoodResponse` — unrelated traffic cannot clear a latched keep-alive error. - `RecoveryAfterBadNoCommunicationIsHandledByTheKeepAliveResponse` — after `BadNoCommunication`, `OnKeepAlive` still enters the recovery branch even though the keep-alive Read's own response passed through `RequestCompleted` first. Local runs: `Opc.Ua.Client.Tests` on net10.0 fully green (2133/2133). `Opc.Ua.Sessions.Tests` (live in-proc reference server) was still running when this PR was opened; I will follow up if it reports anything. ## Related Issues - Fixes #4317 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-08-26 19:19:34 +00:00
Marc SchierandCopilot	b292e243c6	Add trusted NuGet release workflows (#4312) ... ## Summary - replace the preview-only workflow with `nuget-publish.yml`, running on every branch push, `v*` tag, and manual dispatch - build and sign all Release packages plus genuine `.Debug` packages, publish normal packages to GitHub Packages, and retain exact signed package and symbol bytes - add manual `release.yml` promotion from a successful `release/*` publish run to nuget.org through `NuGet/login@v1` trusted publishing - validate package identities, versions, signatures, symbols, duplicates, and SHA-256 hashes before publication and promotion ## Signing configuration All required repository Actions secrets are configured, including `OPCFOUNDATION_NETSTANDARD_SNK_BASE64`, which was transferred directly from Azure DevOps Secure Files without placing the private key in a pipeline artifact. ## nuget.org setup The account-side trusted publishing policy must use: - Repository Owner: `OPCFoundation` - Repository: `UA-.NETStandard` - Workflow File: `release.yml` - Environment: `release` The existing `release` environment contains `NUGET_USER` and is restricted to `release/*` branches. ## Validation - parsed both workflow YAML files against the GitHub workflow schema - parsed all embedded PowerShell and the shared package validator - packed real Release and Debug `Opc.Ua.Types` packages and validated their identities, matching versions, symbols, and `.Debug` family - verified all seven signing secret names and removal of temporary key material --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d639a224-b97c-4dbf-ba98-f99e6e3f6b9a	2026-08-26 16:16:55 +00:00
Copilotandromanett	cb50353b40	Converting code to new source format for configuration loading (#4311) ... Pull request created by AI Agent Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-08-24 16:35:31 +00:00
3 people	496587ef1d	Stabilize macOS Redundancy.Server CI by hardening replication wait timeouts (#4309) ... # Description The `test-macOS-latest-Redundancy.Server` job failed in check run `97081843689` because `ReconcileMaterializedValuesCorrectsStaleReplicaAndRebroadcastsAsync` timed out waiting for inbound replication under runner contention. This PR makes the timeout OS-aware in that test helper so macOS gets a larger convergence window without loosening non-macOS behavior. - **Root cause (from Actions logs)** - `AwaitWithTimeoutAsync` hit `Task.Delay(30s)` first (`replication did not complete within the timeout`) in `ReplicatedAddressSpaceSynchronizerTests`. - **Targeted change** - Replaced hardcoded `30s` waits with a shared helper: - macOS: `90s` - others: `30s` - Applied to both eventual-assert deadline and `Task.WhenAny(...)` timeout in the same test class. - **Code shape** ```csharp private static TimeSpan GetReplicationTimeout() { return OperatingSystem.IsMacOS() ? TimeSpan.FromSeconds(90) : TimeSpan.FromSeconds(30); } ``` ## Related Issues Investigates and addresses CI failure in GitHub Actions check run `97081843689` (`test-macOS-latest-Redundancy.Server`). ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-24 16:11:25 +00:00
3 people	24399f1808	Fix source generator MODELGEN003 diagnostic dropping message placeholders (#4310) ... ## Problem `LoggerAdapter.Log<TState>` in `Tools/Opc.Ua.SourceGeneration/SourceGeneratorTelemetry.cs` reported an **already fully-formatted** logger `message` against a looked-up `DiagnosticDescriptor` whose `messageFormat` may declare more positional placeholders than the single formatted string carries. For `EventId == 3` the descriptor is `SourceGenerator.Exception` (`MODELGEN003`) with: ``` messageFormat = "Exception during model generation '{0}': {1}" ``` That format has **two** placeholders but only **one** argument (`message`) was supplied. Roslyn's `Diagnostic.GetMessage()` runs `string.Format(format, args)`, which throws `FormatException` because `{1}` has no argument; Roslyn's internal catch then returns the **raw, unsubstituted template**. The user therefore saw the literal text `Exception during model generation '{0}': {1}` with the placeholders unreplaced, masking the real error. ## Fix Route the pre-formatted message through a new `CreateFormattedDiagnostic` helper that reports it via a **passthrough descriptor whose `messageFormat` is exactly `"{0}"`**, while preserving the original descriptor's `Id`, `Title`, `Category`, `DefaultSeverity`, `IsEnabledByDefault`, `HelpLinkUri` and custom tags: ```csharp public static Diagnostic CreateFormattedDiagnostic( DiagnosticDescriptor descriptor, string message) { var passthrough = new DiagnosticDescriptor( descriptor.Id, descriptor.Title, "{0}", descriptor.Category, descriptor.DefaultSeverity, descriptor.IsEnabledByDefault, helpLinkUri: descriptor.HelpLinkUri, customTags: descriptor.CustomTags.ToArray()); return Diagnostic.Create(passthrough, Location.None, message); } ``` `LoggerAdapter.Log` now calls this helper instead of `Diagnostic.Create(descriptor, Location.None, message)`. This renders the message verbatim under the correct diagnostic id regardless of the descriptor's placeholder arity, and is safe even when the message itself contains `{`/`}` characters (the format string is only `"{0}"`). > Note: the public `descriptor.CustomTags` (`IEnumerable<string>`) + `.ToArray()` is used because Roslyn's `ImmutableCustomTags` property is `internal`. ### Why the helper lives in `SourceGeneratorHelpers.cs` (not `SourceGenerator.cs`) `SourceGeneratorTelemetry.cs` and `SourceGeneratorHelpers.cs` are shared (glob-linked) into **both** the model generator (`Opc.Ua.SourceGeneration`) and the stack generator (`Opc.Ua.SourceGeneration.Stack`). The stack generator has its **own** `SourceGenerator` partial class and does **not** compile the model generator's `SourceGenerator.cs`. Placing the helper in `SourceGenerator.cs` broke the stack build with `CS0117: 'SourceGenerator' does not contain a definition for 'CreateFormattedDiagnostic'`. `SourceGeneratorHelpers.cs` is the shared partial (where `Guard` also lives), so the helper is available to both generators. ### Guard path untouched The other emission path, `SourceGenerator.Guard(...)` in `SourceGeneratorHelpers.cs`, uses the same `Exception` descriptor but correctly passes **two** args (`ex.Message`, `ex.ToString()`), so that path was already fine and is **not** changed. ## Tests New `tests/Opc.Ua.SourceGeneration.Tests/SourceGeneratorTelemetryTests.cs` (4 tests): 1. **`DirectReportAgainstMultiPlaceholderDescriptorLosesMessage`** — documents the pre-fix behaviour: the raw `Diagnostic.Create(Exception, ..., "boom")` render drops `"boom"` and leaks `{1}`. 2. **`CreateFormattedDiagnosticRendersMessageVerbatim`** — the fix renders `"boom"` exactly, with no `{0}`/`{1}`. 3. **`CreateFormattedDiagnosticPreservesDescriptorMetadata`** — id (`MODELGEN003`), severity, category, help link, IsEnabledByDefault and custom tags are preserved. 4. **`CreateFormattedDiagnosticIsSafeForBracesInMessage`** — a message containing `{`/`}` is emitted verbatim. ## Validation - `tools/Opc.Ua.SourceGeneration` (and the stack variant) build with **0 warnings / 0 errors**. - Full `Opc.Ua.SourceGeneration.Tests` suite on net10.0: **132/132 passing**, including the 4 new tests. ## Files changed - `tools/Opc.Ua.SourceGeneration/SourceGeneratorHelpers.cs` — added `using System.Linq;` + `CreateFormattedDiagnostic` helper. - `tools/Opc.Ua.SourceGeneration/SourceGeneratorTelemetry.cs` — `LoggerAdapter.Log` calls the helper. - `tests/Opc.Ua.SourceGeneration.Tests/SourceGeneratorTelemetryTests.cs` — new regression tests. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-08-24 15:19:26 +00:00
Marc Schier	06ae192084	Fix CI failures tracked by #4299 (#4308) ... ## Summary - stabilize the History, authentication lockout, Robotics, redundancy, and DI tests reported in #4301-#4305 - fix the closed-transport buffered-write completion deadlock that hung `ConnectMultipleSessionsAsync` - add macOS crash diagnostics for #4306 and retain evidence for root-cause analysis - run macOS tests and NativeAOT in GitHub Actions pull requests, using supported Intel and ARM64 runners, while keeping every Azure macOS definition disabled by default - move all remaining source-level `InternalsVisibleTo` declarations into project files so signed OpenUSD builds receive the repository public key ## Local validation - MCP PubSub: 29 passed - History focused tests: 3 passed - Sessions lockout: 3 passed - Sessions multiple-session regression: 3 passed - deterministic binary client channel tests: 22 passed - Robotics runtime: 34 passed - Redundancy focused tests: 4 passed - OpenUSD net10.0: 914 passed - affected OpenUSD projects build on net10.0 and net48 Refs #4299, #4300, #4306 Fixes #4301 Fixes #4302 Fixes #4303 Fixes #4304 Fixes #4305 --------- Copilot-Session: 7cb486de-8b4c-43c5-a900-e7d2e0ad04a8	2026-08-22 19:30:19 +00:00
Copilotandromanett	8ea319d910	Fix macOS ARM64 NativeAOT linker failure in aot-macos-latest (#4298) ... # Description The `aot-macos-latest` job was failing while linking `tests/Opc.Ua.Aot.Tests` on `macos-latest` / `osx-arm64`, before managed tests started. The failure was an Apple linker assertion (`too many large addends`) caused by the size/fixup profile of the NativeAOT test binary. - **Root cause** - `Opc.Ua.Aot.Tests` publishes a large NativeAOT executable. - On Apple Silicon CI, the final native link hit the linker’s addend/fixup limit. - **Change** - Keep the fix scoped to `/tests/Opc.Ua.Aot.Tests/Opc.Ua.Aot.Tests.csproj`. - For macOS only, make the AOT publish size-focused and drop native debug / debugger / stack-trace payload: - `OptimizationPreference=Size` - `DebugType=none` - `DebuggerSupport=false` - `StackTraceSupport=false` - Retain identical-method-body folding. - **Impact** - Lowers linker pressure on macOS ARM64 CI. - Leaves production assemblies unchanged. - Keeps the managed test surface intact; only the CI-only NativeAOT test executable shape changes on macOS. ```xml <PropertyGroup Condition="$([MSBuild]::IsOSPlatform('OSX'))"> <IlcFoldIdenticalMethodBodies>true</IlcFoldIdenticalMethodBodies> <OptimizationPreference>Size</OptimizationPreference> <DebugType>none</DebugType> <DebuggerSupport>false</DebuggerSupport> <StackTraceSupport>false</StackTraceSupport> </PropertyGroup> ``` ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-08-21 15:44:38 +00:00
4 people	de3476d332	Remove HTTPS binding dependency from server packages (#4297) ... # Description `Opc.Ua.Server` and `Opc.Ua.Gds.Server.Common` depended directly on `Opc.Ua.Bindings.Https`, which pulled ASP.NET Core into server applications even when optional HTTPS-based transports were not used. - Remove the HTTPS binding project references from Server and GDS entirely. - Add a Core-owned `IOpcUaTransportBuilder` contract for builders that support transport registration. - Move the type-preserving HTTPS, WSS, Kestrel OPC TCP, and Web API fluent extensions into `Opc.Ua.Bindings.Https`. - Remove the repository-wide `NoHttps` / `NOHTTPS` build switch. - Keep ConsoleReferenceServer HTTPS/WSS support through an explicit binding reference and the ASP.NET Core container runtime. - Document that applications must reference the optional binding package directly. ## Validation - Built `Opc.Ua.Bindings.Https` in Release for all target frameworks. - Built Server and GDS for .NET 10 and .NET Framework 4.8. - Built ConsoleReferenceServer and PumpDeviceIntegrationServer for .NET 10. - Passed 65 HTTPS binding dependency-injection tests. - Passed focused Server and GDS transport-builder tests. - Verified refreshed Server/GDS restore assets and output manifests contain no `Opc.Ua.Bindings.Https` dependency. ## Related Issues Fixes #4296 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce build or analyzer warnings. - [ ] I ran all tests locally using the `UA.slnx` solution against .NET Framework and .NET 10. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <mschier@microsoft.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Copilot-Session: 41b98d43-8bb8-4587-8f3b-9532544dd582	2026-08-21 07:29:18 -07:00
4 people	afbd10e3e3	Expose runtime capability probes to fix TFM-skew test failures (#4282) (#4295) ... # Description The nightly `Test .NETStandard 2.1 Tests (net8.0)` leg compiles the stack as `netstandard2.1` but runs the test assemblies as `net8.0`. Features gated with `#if NET8_0_OR_GREATER` / `#if NET5_0_OR_GREATER` in the stack are therefore absent at runtime, while the tests assume they exist because a `#if` in a test reflects the *test's* TFM (always net8.0 on this leg), not the stack's compiled TFM. This causes the affected tests to fail on that leg. This PR fixes it by exposing stack-side runtime capability probes. Each probe lives in the same assembly as the feature it gates, so its `#if` reflects that assembly's actual compilation. Affected tests now `Assert.Ignore(...)` or branch based on the probe instead of relying on their own compile-time `#if`. ## Approach New public probes: - `DtlsRecordProtection.IsAeadSupported` (+ `IsAeadCipherSuite` made public) - Opc.Ua.PubSub.Udp, net8 gate - `DtlsEcdheKeyExchange.IsRawSharedSecretSupported` - Opc.Ua.PubSub.Udp, net8 gate - `AdditionalEntropyCertificateKeyGenerator.IsEccKeyRegenerationSupported` - Opc.Ua.Server, net5 gate - `HttpsTransportListener.IsWssTransportSupported` - Opc.Ua.Bindings.Https, `NET5_0_OR_GREATER || NETFRAMEWORK` gate (false only for the netstandard2.1 build) Test updates: - DTLS record/handshake/ECDHE tests skip AEAD/raw-shared-secret cases when unsupported, preserving HMAC-suite coverage in parametrized tests. - Server ECC key-regeneration tests skip when unsupported. - The GDS push tests now *validate* the `BadNotSupported` result at runtime via the probe rather than compiling out the assertion, which also preserves the prior net48 behavior. - The ExperimentalAttribute test probes the stack assembly's `TargetFrameworkAttribute` at runtime instead of a meaningless compile-time flag. - The three WSS integration fixtures skip when WSS is unavailable on the loaded runtime. ## Notes for reviewers - On netstandard2.1 the `DtlsRecordProtection` constructor does not throw for AEAD suites; the `NotSupportedException` surfaces later in Seal/Open, so the skip is keyed on the cipher suite being AEAD *and* `!IsAeadSupported`. - The WSS probe gate returns false only for the netstandard2.1 build (which defines neither `NET5_0_OR_GREATER` nor `NETFRAMEWORK`), precisely isolating the build whose legacy Kestrel hosting cannot accept WebSocket connections on a modern runtime. Validated with clean builds (0 warnings, 0 errors) for the changed source projects and for PubSub.Udp.Tests, Server.Tests, Sessions.Tests and Gds.Tests on both net8.0 and net48. ## Related Issues - Fixes #4282 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-08-21 06:24:46 -07:00
4 people	4d71669487	Stabilize macOS Redundancy sample failover tests by isolating demo UDP endpoint (#4292) ... # Description `test-macOS-latest-Redundancy.Samples` was failing because the PubSub demo process exited before emitting the expected failover marker, causing both short-haul demo assertions to time out. This PR hardens the test harness by isolating demo traffic from the default 4840 discovery behavior in constrained CI networking. - **Root-cause containment** - Set `PUBSUB_ENDPOINT=opc.udp://127.0.0.1:4841` in `SampleTestEnvironment.FastDemo` so short-haul demo tests run on a dedicated loopback endpoint. - Keep `HA_INSECURE=true` in the same preset to preserve demo startup behavior. - **Regression guard** - Added `SampleTestEnvironmentTests.FastDemoUsesLoopbackEndpointAndInsecureDemoKey` to lock the test contract for both `PUBSUB_ENDPOINT` and `HA_INSECURE`. - **Code snippet** ```csharp public static IReadOnlyDictionary<string, string> FastDemo { get; } = new Dictionary<string, string>(StringComparer.Ordinal) { ["DEMO_FIRST_SECONDS"] = "1", ["DEMO_SECOND_SECONDS"] = "1", ["PUBSUB_ENDPOINT"] = "opc.udp://127.0.0.1:4841", ["HA_INSECURE"] = "true" }; ``` ## Related Issues CI check-run remediation for failing job `test-macOS-latest-Redundancy.Samples` (job `96263875760`). ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [x] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-21 04:36:34 +00:00
4 people	d525bc3f28	Bound reconnect cleanup and organize samples by domain (#4276) ... # Description This PR fixes bounded ManagedSession reconnect cleanup and reorganizes the sample applications into domain-focused folders. ## Reconnect cleanup - Bounds reconnect integration cleanup with independent 30-second close and dispose limits. - Sends `CloseSession` through the awaited transport request path during `Session.DisposeAsync`. - Preserves explicit `Session.CloseAsync` behavior that waits for or cancels outstanding publish requests. - Covers the disposal request path with a regression test. ## Sample organization - `samples/OpenUsd`: `GeneratorServer`, `SiteCompositionServer` - `samples/Redundancy`: `RedundantClient`, `RedundantServer`, `RedundantPubSub` - `samples/Isa95`: `MinimalIsa95Server` - `samples/MinimalApi`: `MinimalBoilerServer`, `MinimalCalcServer`, `MinimalClient` - `samples/DI`: `PumpDeviceIntegrationServer` - `samples/Lds`: `ConsoleLdsServer` - `samples/Reference`: `ConsoleReferenceClient`, `ConsoleReferenceServer` - `samples/PubSub`: `ConsoleReferencePubSubClient` `UA.slnx` now exposes the same requested domain groups. Project references, tests, runtime sample probes, CI workflows, Dockerfiles, Compose files, helper scripts, and documentation were updated to the new paths. `samples/Robotics/MinimalRobotServer` remains under `Robotics`. ## Validation - `dotnet build UA.slnx` - 3 Redundancy short-haul sample tests - 70 DI sample tests - 914 OpenUSD tests - 1 WotCon model-materialization test - 137 AOT tests - `docker compose config` for all 6 moved Redundancy Compose files ## Related Issues - Fixes #4273 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce new build or analyzer warnings. - [x] I built the complete `UA.slnx` solution locally. - [ ] I ran all tests locally against at least .NET Framework and .NET 10. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <mschier@microsoft.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Copilot-Session: a2c2838f-74ae-4997-a1d4-50574d9e0636	2026-08-21 04:35:47 +00:00