53 Commits

Author	SHA1	Message	Date
romanett	9b034f8291	Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700) ... # Description Nullable reference types are now enabled once, in `common.props`, and every project in the repository builds warning-clean with them. **Configuration** - `<Nullable>enable</Nullable>` lives in `common.props`. All per-project `<Nullable>` settings are removed: 178 project files (enable, annotations, and the `disable` in the three Aot test projects). - 419 per-file `#nullable enable`/`restore` directives and the 4 `#nullable disable` directives are removed. - Kept on purpose: - The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`, `PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the project setting in generated code, so the directive is required there. - `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against the 1.5.378 packages and does not import the repository props. - `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now describe the central setting. **Migration** (the 43 unset and 13 annotations-only projects, roughly 10k diagnostics) - **Production code: annotated at the declarations.** Covers `tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the LDS sample. - Members, parameters and returns are nullable where null actually flows. - Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`, `IsMethodTypeDesign`). - `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take `params object?[]`. `Template.AddReplacement` takes `string?` (rendering already skips null replacements). - `IRegisteredServerStore.RegisterAsync` returns `Task<RegistrationEntry?>`: it returns null when a registration is removed. - Dereferences backed by an invariant (for example, the validated model) use `!`, so runtime behaviour is unchanged. - **Behavioural changes in production code** (small, deliberate): - `NodeSetToModelDesign` throws `InvalidOperationException` when the file is not a NodeSet. Before, it threw a NullReferenceException on the next line. - `ModelDesignValidator` starts its lookup tables empty instead of null. A redundant `if (m_nodes == null) return;` guard is gone. - **Test code: minimal annotations.** - `?` on values that can be null, `!` where the test asserts non-null. - `= null!` for fixture members assigned in SetUp. - `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the `IEncodeable` constraint. - A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`) folded into the adjacent `IsNull`/`Count` checks. **Notes for reviewers** - The diff is large (~1,500 files) but almost entirely mechanical: `?`, `!` and directive removal. The non-annotation changes are listed above. - Known follow-up, not done here: `IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay non-nullable even though the validator returns null before it loads a dictionary. Making them nullable would ripple through every generator call site. ## Related Issues - Fixes #4439 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. The "no new warnings" box covers net10.0 and net48: full non-incremental builds of `UA.slnx`, `tools/SourceGeneration.slnx`, `tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0 warnings and 0 errors under the repository's warnings-as-errors settings. net9.0 and net8.0 also had no nullable diagnostics, but they were not rebuilt after the final fixes. Tests have not been run locally yet. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-08 08:48:13 +02:00
romanett	cce2004fdd	CTT: keep GeneratesEvent off Object instances and Views off non-hierarchical references; classify Deadband Err-006 as CTT defect (#4667) ... # Description Three findings from the 2026-10-03 CTT run with the patched 1.05.513 scripts. Two are server/model defects and are fixed here. The third is a CTT script defect, recorded as C56 in `plans/ctt-issues.md`. ## 1. Monitored Items Deadband Filter `Err-006.js`: CTT defect (C56, no server change) The script modifies a MonitoredItem on the first static numeric scalar (a plain BaseDataVariable with no EURange) to use a PercentDeadband. It accepts only `Good` or `Bad_MonitoredItemFilterUnsupported`. Since #4629 the server returns `Bad_DeadbandFilterInvalid`, and that is the code the spec defines for this case: - OPC 10000-8 §7.2: PercentDeadband "applies only to AnalogItems with an EURange Property". - OPC 10000-8 §7.3.2 Table 61: `Bad_DeadbandFilterInvalid` is returned when "a PercentDeadband is not supported, since an EURange is not configured". CreateMonitoredItems and ModifyMonitoredItems both go through `ValidateMonitoringFilterAsync` and return the same code. Monitor Basic `Err-022.js` already accepts `Bad_FilterNotAllowed` for non-analog items, so the scripts disagree with each other here. Draft Mantis note (not filed yet): > **Monitored Items Deadband Filter Err-006.js rejects Bad_DeadbandFilterInvalid for a PercentDeadband on a node without EURange** > Err-006.js picks the first node of `/Server Test/NodeIds/Static/All Profiles/Scalar/NumericSettings`, which is normally a BaseDataVariable without an EURange. It then calls ModifyMonitoredItems with a PercentDeadband (10 %) and accepts only Good or Bad_MonitoredItemFilterUnsupported. OPC 10000-8 §7.2 limits PercentDeadband to AnalogItems with an EURange. §7.3.2 Table 61 defines Bad_DeadbandFilterInvalid for "a PercentDeadband is not supported, since an EURange is not configured", so a compliant server fails this test. Monitor Basic Err-022.js already accepts Bad_FilterNotAllowed for the non-analog fallback. > Proposed fix: add StatusCode.BadDeadbandFilterInvalid (and, consistent with Err-022.js, StatusCode.BadFilterNotAllowed) to the ExpectedAndAcceptedResults of the ModifyMonitoredItems call. ## 2. GeneratesEvent on Objects (Base Info Core Structure 2 `001.js`): source generator defect The CTT reported `ns=9;i=234` (the `<ServiceName>` placeholder of AuthorizationServicesFolderType) and `ns=9;s=AuthorizationServices/Default` as Objects that are the source of GeneratesEvent references. OPC 10000-3 §7.15 says: "The SourceNode of References of this type shall be an ObjectType, a VariableType or a Method". `OpcUaGdsModel.xml` declares both references (AccessTokenRequested/Issued audit events) correctly, on `AuthorizationServiceType`. The source generator copied the type's own non-hierarchical references onto every Object or Variable built from the type: onto instance declarations (the placeholder) and into `CreateInstanceOf<Type>`, which `AuthorizationServiceState` initialises from, and that is how the `Default` service gets them. UA-ModelCompiler does the same thing, so the official `Opc.Ua.Gds.NodeSet2.xml` in UA-Nodeset has the same two references on `ns=1;i=234`. Fix: - `TranslateReferences` now marks references declared on a type (`HierarchyReference.DefinedOnType`, which already existed but was never set). - `NodeStateGenerator.GetReferences` drops forward GeneratesEvent references (including subtypes such as AlwaysGeneratesEvent) for Object and Variable nodes when the reference came from the type. - ObjectTypes, VariableTypes and Methods keep these references. For example, the RegisterApplication methods and the Acknowledge/AddComment methods of the alarm types are unchanged. - References that a model declares explicitly on an instance are emitted as before. ## 3. View targeted by HasCause/HasEffect (Base Info Core Structure 2 `001.js`): sample model defect #4485 added `Views_Operations HasCause/HasEffect Views_Engineering` so that View Minimum Continuation Point 01 `012.js` and View Basic 2 `018.js` have at least two references to View nodes on the configured View (`/Server Test/NodeIds/NodeClasses/View` = `Views_Operations`). OPC 10000-3 §5.4 says Views "can be the SourceNode of any hierarchical Reference. They shall not be the SourceNode of any NonHierarchical Reference." `Views_Operations` now Organizes `Views_Engineering` and a new View, `Views_Maintenance`, which organizes `DataAccess`. This keeps two View-class references and adds no loops (ContainsNoLoops stays true). ## Tests - `NodeStateGeneratorTests.TypeGeneratesEventReferencesAreNotCopiedToObjectInstances`: covers the type, the placeholder, the instance, `CreateInstanceOf`, Methods and an explicit instance reference. It fails without the fix. - `ReferenceServerTest.SampleViewsHaveNoNonHierarchicalReferencesAsync`. The existing `NodeClassSampleNodesReferenceTheirOwnNodeClassAsync` still passes. - Results with net10.0: - SourceGeneration.Core.Tests: 4267 passed, 0 failed - InformationModel.Tests: 1078 passed, 0 failed - Di.Tests, including RobotOpenUsdE2e: 438 passed, 0 failed - OpenUsd.Tests: 921 passed, 0 failed - Server.Tests `ReferenceServerTest`: 56 passed, 0 failed - `UA.slnx` builds with net10.0. The changed test projects build with net48. ## CTT (patched 1.05.513 scripts, compared with the 2026-10-03 patched baseline) | Part | Change | | --- | --- | | p02-base-info | Base Info Core Structure 2 `001.js`: Error → Skipped. The GeneratesEvent and View errors are gone; the remaining skips are the CTT's own "type not defined within the test-script" notes. No other change. | | p03-base-view-method | No change. View Basic 2 `018.js` and View Minimum Continuation Point 01 `012.js` still pass. | | p13-monitored-items | Deadband `Err-006.js` still Error (C56). With the script fix it passes. Monitor Basic `007.js` gave a response-timestamp delay warning (>200 ms) caused by machine load; it also appears in the 2026-09-29 base run. | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-05 05:50:30 +00:00
romanett	61edaa1ed0	Remove netstandard2.1 and net472 target frameworks (#4624) ... # Description Drops support for **netstandard2.1** and **.NET Framework 4.7.2**. Libraries now target `net48`, `net8.0`, `net9.0` and `net10.0`. `Opc.Ua.Types` and `Opc.Ua.SourceGeneration.Core` still also target `netstandard2.0`, but only because the Roslyn source generators load them. There is no general netstandard2.0 support. **Preprocessor directives** (112 files) - Every `#if`/`#elif` mentioning `NETSTANDARD2_1*`, `NET472*`, `NET_STANDARD_TESTS`, `SKIP_ECC_CERTIFICATE_REQUEST_SIGNING` or `NETSTANDARD*` was evaluated against the remaining TFM set (net48, net8/9/10, plus netstandard2.0 inside `Opc.Ua.Types`/`tools`) and simplified. Dead branches were removed. - Example: `NETSTANDARD2_1 || NET8_0_OR_GREATER` → `NET8_0_OR_GREATER`. - Example: `NET8_0_OR_GREATER || NET472_OR_GREATER` → removed (always true). - Netstandard-only code was removed: `UnixFileModeShim`, the CRDT reflection path in `ReplicatedGossipOptions.CreateReaderOptions`, and the netstandard2.1-only test variants. **Custom test logic** - `targets.props` drops the `net472`, `netstandard2.0` and `netstandard2.1` `CustomTestTarget` profiles, `NetStandardTests` and `SKIP_ECC_CERTIFICATE_REQUEST_SIGNING`. Supported values are now `net48`, `net8.0`, `net9.0`, `net10.0`. - The `NET_STANDARD_TESTS` define is removed from 29 test projects. - The "build netstandard shells as net10.0" overrides are removed from 18 projects. - `RestrictForLegacyTfm` now only applies to `net48`. - `tests/customtest.bat` is updated to the new values. **Build / packaging / CI** - MSBuild TFM conditions are simplified: Kestrel package choice, MQTTnet v4 (net48 only), BouncyCastle, Di/OpenUsd/Vision/Stress test excludes, and `System.IO.FileSystem.AccessControl` (dropped from Core). - The MigrationAnalyzer nuspec drops its net472/netstandard2.1 lib groups. The Azure signing lists are updated. - `get-ci-matrix.ps1` removes the net472/netstandard test profiles (including the PR-gating `linux-netstandard2.1`) and the matching build legs. **Breaking API change (2.0 preview):** removed `HttpsTransportListener.IsWssTransportSupported` and `ReplicatedGossipOptions.IsGossipStateDecodingSupported`. They only returned `false` on netstandard2.1 and are now always `true`. The test guards that used them were removed as well. **Docs** - NuGet READMEs, `README.md`, `DeveloperGuide.md`, `PubSub.md`, the certificate/ECC/identity docs and the 2.0 migration guide (plus its `.agents` skill mirror) are updated. - The migration guide notes that 4.7.2 consumers must move to 4.8 and netstandard2.1 consumers to net8.0+. ### Reviewer notes - The `System.IO.FileSystem.AccessControl` `PackageVersion` pin is left in `Directory.Packages.props`. It has no direct reference any more, but transitive pinning is on, so it was kept to avoid shifting resolved versions. - The `#if` rewrite was done mechanically by a truth-table evaluator. The diff in the `.cs` files is limited to directive lines, removed dead branches, and stale-comment updates. ### Verified locally - `dotnet build UA.slnx -p:CustomTestTarget=net48`: 0 errors. - `dotnet build UA.slnx -p:CustomTestTarget=net10.0`: 0 errors. - Default multi-target builds (no `CustomTestTarget`) of Opc.Ua.Types, Bindings.Https, PubSub.Mqtt and Redundancy.Server succeeded. - net10.0 tests: - Opc.Ua.Redundancy.Server.Tests: 840/840 passed. - Opc.Ua.Types.Tests: 11363 passed, 1 skipped. - ModelDependencyScanner tests: 20/20 passed. - The full test suite was **not** run locally. ## Related Issues - None ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-02 17:09:49 +02:00
Klaus Landsdorf	ac8dc531ee	Source generator, ISA-95 Job Control and DI changes for companion SDKs built on referenced models (#4577) ... ## Summary First of four stacked PRs that contribute complete implementations of the Machinery (OPC 40001-1 with Job Management, Result Transfer, Process Values and Energy), Pumps (OPC 40223) and Scales (OPC 40200, with PackML OPC 30050) companion specifications — tracked in #4576. It follows the split used for Robotics (#4123–#4127): this PR carries only the changes to **existing shared code** those SDKs need, and adds no project. Each change stands on its own and is covered by tests in this PR; the companion models are what surfaced them. Rebased onto master after #4569. The one conflict was in `NodeSetToModelDesign.cs`: #4569's shared `AssignSymbolicIds` already makes `Import` and `GetImportedSymbols` derive the same ids, and `NormalizeParentNodeId` moves into its pre-pass. The Copilot review is addressed in the last six commits (see "Review follow-up"). ## Changes ### Source generator (`tools/Opc.Ua.SourceGeneration.Core`) - **VariableType data type across assemblies.** A model that consumes a dependency through a referenced assembly's `[ModelDependency]` payload lost a VariableType's `DataType`/`ValueRank`, so `GetNodeStateClassName` dereferenced null and reported `MODELGEN003`. First hit: DI's `LifetimeVariableType` as used by Machinery. - **Method declaration ↔ method state identity.** A consumer re-declaring an inherited method referenced an `OwnerType_MethodMethodState` class the producer never generated. First hit: OPC 34100 ECM re-declaring the DI `LockingServices` methods. - **Browse names and reference types of inherited children.** An inherited child was emitted with `ReferenceTypeId = NodeId.Null` (invisible to a filtered Browse over the wire) and, where it lives in another namespace, with the declaring model's namespace. When the carried namespace is one the consuming model does not declare, the generated code names it by URI instead of emitting an empty `GetIndexOrAppend()`. - **Payload compatibility.** The three additions above travel in a length-prefixed, versioned declaration trailer after the existing trailers, so the node and child records keep their layout: an older reader ignores the capability bit and stops after the trailers it knows, and a payload without declarations is byte-identical to before. - **Symbolic ids of children listed before their parent.** Every `ParentNodeId` is normalized before any symbolic id is derived. No NodeSet in the repository has such an ordering today, so no generated symbol changes; OPC 30050 PackML (later in the stack) declares seven method `InputArguments` before their methods and failed with `MODELGEN025`. - **Methods overridden with another method-state class.** The subtype declares its own field and property for such a method, but `GetChildren` and `RemoveExplicitlyDefinedChild` skipped every overridden child and the supertype's field stays empty, so the method was missing from every instance (not browsed, no per-instance NodeId, not callable). The existing `MethodTypeOverride` test model has the gap; since #4555 generates a method with inline arguments as `MethodState`, OPC 40001-101 `ResultTransferType.GenerateFileForRead` hits it. It also affects models already in the repository: DI `SoftwareFolderType.Delete`, ISA-95 V2 `JobOrderReceiver.Update`, WoTCon `WoTDocument.Validate` and XRegistry `Group.Delete` / `Resource.Delete` now appear on their instances; the DI, ISA-95, WoTCon and XRegistry tests pass. ### ISA-95 Job Control (`src/Opc.Ua.ISA95.Server`) - **`Isa95JobControlV2Binder`** — the V2 receiver/response-provider binding moves out of `Isa95NodeManager` so OPC 40001-3 JobManagement can bind the same eleven verbs on its own instances instead of copying them. `Isa95NodeManager` behaviour is unchanged. - **`Isa95AddressSpaceOwnership`** — a second registration that would load the ISA-95 models fails immediately and names the current owner, as `DiAddressSpaceOwnership` already does for DI. - **`IIsa95JobResponseCatalog`** — a source for `JobOrderResponseList`, which the two state-filtered request methods cannot express. - **`IIsa95JobResponseChangeSource`** — a change stream of received responses, and of responses the provider drops after `ResponseRetention`; a response changes neither the order's state nor the order catalog, so the existing streams cannot report it. Both interfaces are implemented by `InMemoryIsa95JobControlProvider`, registered by `AddInMemoryIsa95JobControlProvider`, and count in `AddIsa95Server`'s provider detection and mixed-provider rejection. ### Device Integration (`src/Opc.Ua.Di.Server`, `src/Opc.Ua.Di.Client`) - **`IDiPostSetupContext.GetService<T>()`** — an optional lookup next to `GetRequiredService<T>()`, so a companion SDK can probe for a seam the server may not have filled without catching `InvalidOperationException`. The context still exposes no raw `IServiceProvider`. - **`DiNodeManager.ConformanceUnits` / `ServerProfiles` are virtual**, so a companion subclass appends the units its own model satisfies. - **DI client browse enumerators follow `BrowseNext`.** `DiTopologyClient.BrowseChildrenAsync` and `DiDeviceClient.BrowseFunctionalGroupsAsync` discarded the continuation point and returned an incomplete list from a paginating server. A bad status for the browsed node still means "no children" and a failed service call propagates; an outstanding continuation point is released whenever the enumeration stops early. ## Review follow-up | Copilot finding | Commit | |---|---| | Payload additions break older readers (both High) | `fix(sourcegen): move the dependency declarations into a trailer` | | Browse failures reported as empty, broad exception filter (Medium ×2); missing pagination tests (Low ×2) | `fix(di-client): let a failed browse call propagate` | | New provider facets missing from detection (Medium) | `fix(isa95): count the response facets as Job Control providers` | | Second review: continuation point kept after a failed BrowseNext | `fix(di-client): release the continuation point after a failed BrowseNext` | | Second review: purged responses not reported | `fix(isa95): report dropped job responses on the response change stream` | | Second review: missing V2 namespace cast to 65535 | `fix(isa95): reject a missing Job Control V2 namespace in the binder` | ## Points for the reviewer - `GetService<T>()` adds a member to a public 2.0 interface. The only implementation is the internal `DiPostSetupContext`; an external implementer would have to add it. - `DiDeviceClient.BrowseFunctionalGroupsAsync` used to throw for a bad status of the device node; like `DiTopologyClient` it now yields nothing. - The commits are self-contained and ordered; reviewing commit by commit is the easiest route. ## Testing - New and extended tests: `ModelDependencyV1Tests`, `NodeStateGeneratorTests`, `NodesetIdentifierSidecarTests`, `InMemoryIsa95JobControlProviderV2Tests`, `Isa95ServerBuilderTests`, `Isa95JobControlV2BinderTests`, `DiDeviceClientTests`, `DiTopologyClientTests`, `OpcUaServerDiBuilderExtensionsTests`. - `UA.slnx` builds for net472, net48, netstandard2.0/2.1 and net8/9/10 with SDK 10.0.401 and no warnings. - Tests on net10: SourceGeneration.Core 4266 (8 skipped), SourceGeneration 263, ISA95 144, DI 433; the first three also on net8. The changed projects build for net48 and netstandard2.0 without warnings. - Changed-line coverage (`check-coverage.ps1`): 91.1 % (594 of 652 lines). - The companion stack on top (#4578, #4579, #4580) is rebased onto this head. - Fork CI, full PR matrix incl. macOS, for this head (running): https://github.com/biancode/UA-.NETStandard/actions/runs/36558506011. CodeQL: https://github.com/biancode/UA-.NETStandard/actions/runs/36558509242. The previous head (before the second review round) passed every Windows and Linux job: https://github.com/biancode/UA-.NETStandard/actions/runs/36542929585.	2026-09-29 14:50:00 +02:00
romanett	20c2d77d57	Fix Core/Types audit findings and conform them to the OPC UA spec (stacked on #4569) (#4573) ... > **Stacked on #4569.** The base branch is `romanett/practical-murdock-fd9c56`, so this diff shows only the commits of this PR. Retarget to `master` after #4569 merges. ## Summary Bug audit of `Opc.Ua.Core`, `Opc.Ua.Core.Types`, `Opc.Ua.Types` and `Opc.Ua.Core.Schema`, with the findings fixed and the fixes grounded against the OPC UA specification (reference.opcfoundation.org). 133 commits in four groups: 1. **Audit fixes** (`Fix <ID>: ...`): 116 findings from a read-only audit, each with a regression test. IDs follow the audit scopes: T1-T6 `Opc.Ua.Types`, C1-C4 `Opc.Ua.Core`, S1 `Opc.Ua.Core.Types` + `Opc.Ua.Core.Schema`. 2. **Review fixes** (`Fix review <ID>: ...`): issues a code review of group 1 found, plus lower-severity leftovers. 3. **Spec fixes** (`Spec: #<n> ...`): the 18 places where an earlier fix only partly matched the spec text. 4. Test and analyzer follow-ups. Where `master` (#4516, #4522, #4540, #4564) or #4569 already fixed the same issue, their version was kept and this PR only adds what they didn't cover. ## Behaviour and user-facing changes ### Wire format and interoperability (breaking) - **PubSub AES-CTR (Aes128-CTR / Aes256-CTR)** now follows Part 14 Tables 156/157: - The counter block is `KeyNonce[4] | MessageNonce[8] | BlockCounter[4]`, and the block counter starts at 1. - The SecurityHeader nonce is 8 bytes: `Random[4] | UInt32 SequenceNumber`, with the sequence number starting at 1. - SKS key data is split with a 4-byte KeyNonce, giving 52 / 68 bytes. - **Not interoperable with earlier releases of this stack**, but matches the spec. There is a test against the independent RFC 3686 vector. - **User token signature** (Part 4 6.1.8): - The signed data is now built from the user token policy's SecurityPolicy on both client and server. - It also supports the SecurityMode None layout `ServerNonce | HASH(ServerCertificate) | ClientNonce`. - New overload `SecurityPolicyInfo.GetUserTokenSignatureData(..., MessageSecurityMode)`. - **Secure channel:** - The ECC AEAD `LastSequenceNumber` is correct across the 32-bit sequence wrap (Part 6 6.8.1). - The incoming chunk counts against MaxChunkCount/MaxMessageSize. - The sender no longer lets a message through that is 24 bytes over MaxMessageSize. - The OPN response signature is verified without overrunning the buffer (RSA_DH with differing key sizes). ### Sessions (server) - **CreateSession nonce:** - The client nonce is kept on None channels, because the None-mode user token signature covers it. - A nonce over 128 bytes returns `Bad_NonceInvalid` on every channel (Part 4 5.7.2.3). - The duplicate-nonce check runs only on secured channels. - A non-empty nonce under 32 bytes is still accepted on None, for compatibility. - **ActivateSession:** an embedded `EccEncryptedSecret` signing certificate is validated with the server's peer certificate validator (Part 6 6.8.3). Byte equality with the client certificate is only the fallback when no validator is available. New opt-in interface `ICertificateValidatorProvider`. - **FindServers / GetEndpoints:** `GatewayServerUri` and `DiscoveryProfileUri` are no longer swapped. ### Write, Read and Call (NodeState / node managers) - **UserWriteMask:** - Enforced for non-value attributes and for VariableType Value writes (`ValueForVariableType`), returning `Bad_UserAccessDenied`. - An unconfigured UserWriteMask (0, no handler) is not enforced, and reads back as the WriteMask so Read and Write agree (Part 3 8.60). - A throwing or Bad `OnReadUserWriteMask` handler fails only that item. - **Index-range writes:** - Slices are merged into the cached value under the attribute lock, also for `OnWriteValueAsync` handlers. - A data-change notification is still produced when the cache cannot absorb the slice. - The slice must match the unclipped range, otherwise `Bad_IndexRangeDataMismatch` (new status code in `Opc.Ua.Types`). - A range past the target returns `Bad_IndexRangeNoData`. - A handler without index-range support returns `Bad_WriteNotSupported` (Part 4 7.27, 5.11.4.4). - **Index-range reads on matrices** return partial results clipped to the source, instead of allocating the requested size (a Read with `"0:40000,0:40000"` no longer allocates about 1.6e9 elements). - **DataTypeDefinition writes:** - A value that isn't a decoded DataTypeDefinition, or whose kind doesn't match the DataType category, returns `Bad_TypeMismatch`. - Null is rejected for concrete Structure/Enumeration/OptionSet types. - WriteMask is checked first (Part 3 5.8.3). - **AnalogItem:** a null write skips the InstrumentRange check again. - **Method call:** the async fallback passes the ObjectId to the method handlers. - **Redundancy:** `DistributedValueParticipation` merges index-range writes and applies the index range on reads. - **Change masks:** `ClearChangeMasks` takes the bits atomically, so concurrent changes are no longer lost. - **`AccessLevelEx`:** preserved by Clone/Export. ### Alarms & Conditions (Part 9) - **Acknowledge / Confirm / AddComment:** - Accept any EventId the condition or its branches reported while that state is still pending (5.7.3). - An unknown id returns `Bad_EventIdUnknown`. - An already handled state returns `Bad_ConditionBranchAlreadyAcked` / `...AlreadyConfirmed`. - AddComment on a branch no longer changes the trunk, and the branch table is re-keyed when a branch's EventId changes. - **Shelving:** - OneShotShelve without MaxTimeShelved no longer overflows the timer. - A timed shelve is measured on the monotonic clock, so wall-clock steps don't delay the unshelve. - The state is named `OneShotShelved`. - **Audit events:** - They report failure for rejected method calls. - `SelectedResponse` is a UInt32. - A rejected negative Respond leaves SelectedResponse unset. - **`ContentFilterElement.Validate`:** an unknown FilterOperator is reported per element as `Bad_FilterOperatorInvalid`, instead of failing the whole service call. ### ContentFilter evaluation (Part 4 7.7.3, on top of #4564) - **Bitwise operators:** - The result takes the size of the larger operand. - Boolean, numeric String and one-element array operands are converted to integers implicitly. - Types without an implicit integer conversion give NULL. - **Strings:** ordering is ordinal, no longer culture dependent. - **Cast:** restricted to the Table 121 conversions, and QualifiedName to LocalizedText is added. The general `Variant.ConvertTo` keeps its wider set. - **Null and empty operands:** - `Like` with a NULL operand is NULL. - Empty arrays of the same DataType compare equal. - Null-string operands are NULL everywhere. ### Encoders and built-in types - **XML:** - Strings keep leading, trailing and whitespace-only content (Part 6 5.3.1.5). NodeSet import opts back into treating whitespace-only strings as empty with the new `XmlDecoder.TreatWhitespaceOnlyStringsAsEmpty`. - Layout whitespace around `<Identifier>` values is ignored, and trailing control characters are stripped from `s=` ids. - `XmlElement` ignores DTDs without expanding entities. - **JSON:** - A DateTime without an offset is read as UTC. - An unmappable `nsu=` NodeId and an unknown `svu=` ExpandedNodeId decode to a namespace-0 String NodeId (5.4.2.10 / 5.4.2.11). - `svr=` is rejected in a NodeId field. - Bad matrix dimensions return `BadDecodingError`. - The non-strict body stack stays balanced after an error. - The nesting limits match between encoder and decoder. - **Binary:** - `MaxByteStringLength` no longer applies to strings. - ExtensionObject bodies count toward the nesting limit on seekable streams (and in the XML encoder). - **Parsing:** - The NodeId parser requires `i=`/`s=`/`g=`/`b=`. - `ExpandedNodeId.Parse` rejects an empty `nsu=`/`svu=`. - NumericRange rejects a trailing `,` with `Bad_IndexRangeInvalid`. - RelativePath escapes `#`. - **Status codes:** `StatusCode.WithSubCode`/`WithCodeBits` keep the severity bits and look up the symbolic id for the new code. - **Variant / VariantHelper:** - `TryCastTo` returns false on a type mismatch. So `DataValue.GetValue<T>(default)` returns the default, and `GetValueOrDefault` / `CastTo` throw `BadTypeMismatch` as documented. - `System.DateTime` casts are supported. - Concrete encodeable arrays (e.g. `Argument[]`) no longer throw. - `ConvertTo` keeps one-element and empty arrays as arrays and checks the conversion is legal for empty arrays. - `CastFrom(object[])`, `CastFrom(Enum[])` and the obsolete `Variant(object)` constructors keep the array shape (previously a one-element array collapsed to a scalar and an empty one to Null). - `TypeInfo.Unknown` / `Variant.Null.TypeInfo` report `ValueRanks.Any`. - **Equality and hashing contracts fixed:** - Types: Matrix/MatrixOf, typed-null Variants, NodeId (the namespace index is now hashed), ExpandedNodeId, LocalizedText, DiagnosticInfo, StructureDefinition/EnumDefinition, CertificateIdentifier, UserIdentity and DateTimeUtc. - `MatrixOf.Dimensions` returns a copy. - Decimal equality and hashing now run in bounded time. - **Localized text:** a LocalizedText with locale `"mul"` and no usable translations, or null client locale ids, no longer throw. ### NodeSet, address space and utilities - **NodeSet export/import:** - Server index 0 is the local server on import and export, so local ids are no longer written as `svr=65535`. - `StructureDefinition.BaseDataType` is filled from the supertype on import. - **`NodeTable.Import`:** replaces duplicates, keeping the references neighbouring nodes hold. `TypeTable.Add` detaches a re-parented type from its old supertype, and `Node.Clone` gets its own ReferenceTable. - **`StringTable` / `NamespaceTable`:** capped at UInt16 indexes (`BadEncodingLimitsExceeded`) for Append and Update, instead of silently wrapping. - **Small fixes:** - `UnsecureRandom.Shared` is seeded randomly. - The peer's RetryAfterMs is clamped to one day. - The `CancelAsync` and `string.GetHashCode(StringComparison)` polyfills match BCL behaviour. - `CoreUtils.IsEqual` compares strings ordinally. ### Schema generation and ComplexTypeSystem - **JSON schema:** describes the 1.05 layout with the compact/verbose null rules, and now validates real `JsonEncoder` output. - **XSD:** - `EncodingMask` (`xs:unsignedLong`) only for StructureWithOptionalFields. - Guid and StatusCode are complex types. - Standard types are in the `Types.xsd` namespace. - **Structure-backed OptionSets** are described as `{Value, ValidBits}`. New `UaTypeDescription.IsStructureOptionSet` property and constructor overload. - **ComplexTypeSystem:** - A malformed type in a server dictionary is skipped per type instead of discarding the whole dictionary. - Cyclic supertype walks are bounded. - Declared definitions are no longer changed. - A dictionary validator is kept only if the dictionary parsed. ### Certificates and trust lists (Part 4 6.1.3, Part 12) - **Revocation:** - Checked for issuers supplied in the peer's chain and for inline-trusted issuers, against the CRLs in the trusted and issuer stores. - A store without CRL support no longer hides "revocation unknown". - `RejectUnknownRevocationStatus` is honoured for inline-trusted issuers unless the entry sets `SuppressRevocationStatusUnknown`. This changes the strict case of `ExplicitIssuerChainIsHonoredWithoutStorePathAsync`. - **Basic constraints / path length** violations fail with `Bad_CertificateInvalid`. - **`TreatAsInvalid`:** - Now yields `Bad_CertificateUntrusted`, which can't be suppressed or auto-accepted. - It is honoured on individual trust-list entries even when the certificate is also in a store. - **`EccEncryptedSecret`:** enforces the result of validating the sender certificate. - **`TrustListTransaction`:** - Routes each CRL to the store holding its issuer (TrustedCrls/IssuerCrls). - Validates staged CRLs, including the store's CRL support, before changing anything. - Applies certificates before CRLs. - **Other:** - One failing `CertificateChanges` observer no longer stops the others. - `InMemory:` stores share their contents per path, and `InMemoryStoreProvider` is `IDisposable`. ### WoT (`Opc.Ua.Types/Wot`) - **Projection security:** projection security schemes whose qualified names collide are refused. New diagnostic `WotDiagnosticCode.ProjectionSecurityConflict = 6063`. - **Input handling:** - Invalid Unicode (lone surrogates, bad UTF-8) is rejected as `JsonException`. - External-schema results are keyed per affordance. - A duplicate security-floor member is rejected. - `nsu=` ids are percent-encoded (Part 6 5.1.12). - The configured JSON depth is honoured on re-parse. - An oversized document is reported before it is parsed back. - **Performance and limits:** - Subtype-graph validation, `componentOf` linking and the namespace table are now linear. - Namespaces are capped at UInt16. ### New or changed public API - `ICertificateValidatorProvider` (server) - `SecurityPolicyInfo.GetUserTokenSignatureData(..., MessageSecurityMode)` overload - `XmlDecoder.TreatWhitespaceOnlyStringsAsEmpty` - `UaTypeDescription.IsStructureOptionSet` plus a constructor overload - `WotDiagnosticCode.ProjectionSecurityConflict` - `StatusCodes.BadIndexRangeDataMismatch` in the `Opc.Ua.Types` list - `InMemoryStoreProvider : IDisposable` ## Not changed / open - **Deferred:** - S1-14 (BSD OptionSet size needs the base type in `UaTypeDescription`, and BSD is deprecated). - C2-7 (restoring the own-certificate trust exemption would accept certificates the spec says to reject; removing the dead code is a separate decision). - A1-4 (config values keep surrounding whitespace, as the pre-April-2026 loader did). - H-2 (JsonDecoder has no real nesting counter). - A2-3 (the StringTable cap also applies to diagnostics tables; default `MaxArrayLength` already bounds them). - **Needs a product decision:** - The general `ConvertToString` writes a QualifiedName as `ns:Name`, while Table 121 drops the namespace. - Short (< 32 byte) client nonces are accepted on None channels. ## Verification - Every commit added a regression test that fails without its fix, where practical. - Before the rebases, with all audit and review fixes (before the `Spec: #…` commits): 0 build warnings and 16 test suites green on net10.0. The suites: Types, Core, Core.Encoders, Core.Security, Core.Schema, Client, Client.ComplexTypes, Server, Sessions, Gds, History, Features, Redundancy.Server, PubSub, WotCon and WotCon.Bindings. Core hit its known intermittent end-of-run hang once, and reruns were clean. - The `Spec: #…` fixes were verified per group on their own suites. Those include PubSub.Udp, Redundancy.PubSub, PubSub.Server, Configuration and SourceGeneration.Core. - **Not yet built or tested on this base.** It was rebased onto master (resolving overlaps with #4522, #4540 and #4564) and then onto #4569. This PR stays a draft until the full run on this base is green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>	2026-09-29 11:14:15 +00:00
romanett	442889cba5	Fix source generator audit findings; conform structure-field matrices and type metadata to OPC UA 1.05 (#4569) ... # Description This PR fixes a bug audit of the source generation projects (`tools/Opc.Ua.SourceGeneration*`: model loading, NodeSet import, all emitters, the Roslyn hosts and packaging). It also fixes a max-effort code review of those fixes, and a check of every spec-relevant change against the OPC UA 1.05 text. - **Scope:** about 85 audit findings, 48 review findings and 32 spec-grounding items were addressed. Each fix has a regression test that fails without it. - **Wire change:** it includes one deliberate, stack-wide wire change: **structure-field matrices now follow OPC 10000-6 5.2.5 Table 28**. Most changes are internal correctness fixes. The sections below list every change a user can observe: wire formats, generated code and API, generated address-space content, diagnostics and runtime behaviour. ## Behaviour / user-facing changes ### Wire format (encoders) - **Binary inline matrices** (OPC 10000-6 5.2.5 Table 28). - **Layout:** a multi-dimensional *structure field* is encoded as `Int32[] Dimensions`, then the values **without a length prefix**, with at least 2 dimensions. - **Empty and null:** any dimension <= 0 means no values follow, and a null matrix is `-1`. An empty matrix is written as `[0,0]`. - **Compatibility:** this is **not compatible with earlier builds of this SDK** for matrix fields. It applies to generated types, `[DataType]` types, `Structure`, client ComplexTypes and `WriteEncodeableMatrix`. - **Unchanged:** Variant matrices (5.2.2.16) keep their old encoding. - **Declared rank decides matrix vs array.** A 1-D `MatrixOf<T>` in an array field or a Variable value is written as an array again. Matrix-declared fields go through the new public `EncoderExtensions.WriteInlineMatrixValue`, which generated code also uses. A non-empty 1-D value in a matrix field throws `BadEncodingError`. - **Decoder hardening:** - Before allocating, the decoder bounds all matrix dimensions against overflow, `MaxArrayLength` and the remaining message length. - The decoder rejects: - a matrix with fewer than 2 dimensions; - a populated matrix whose rank differs from the field's declared rank; - a union SwitchField greater than the number of fields; - an EncodingMask with unassigned bits set. - **Encoder `MaxArrayLength`:** the encoder enforces `MaxArrayLength` for inline matrices, as it already did for arrays. - **XML structure fields** (5.3.1, 5.3.4, 5.3.5). - **Fields:** `Structure` and client ComplexTypes write fields like typed fields (`<A>1</A>`, array elements directly) instead of a Variant body. - **Matrix fields:** `Dimensions` and `Elements` go directly inside the field element, with no `<Matrix>` wrapper. - **Empty matrices:** written as nil, because XML dimensions must be greater than 0. - **Compatibility:** decoders still accept the old shapes. - **Enum fixes:** enum array elements with value 0 are no longer dropped, and enum array element names are fixed. - **Other encoding fixes:** - A null `DiagnosticInfo` structure field writes `0x00` instead of nothing. - A null scalar field in a runtime `Structure` is written as its default instead of zero bytes, which used to shift every following field. - A Variant holding an empty matrix encodes as an empty array instead of throwing. - `Variant.Copy` keeps the matrix identity of null and empty matrices. - **NodeSet export and the DataContract surrogate** write Variable values with the Variant rules again. A null or empty `MatrixOf` variable no longer produces invalid XML or unreadable NodeSets. - **`JsonEncoder.CanOmitFields`** is `true` only for Compact encoding. Verbose and RawData JSON include default-valued fields. ### `[DataType]` source-generated types - **Default-value handling** (OPC 10000-6 5.4.1, Table 1). - **Encode:** a field is omitted only when its value *and* its declared default are the type's default (0, false, null). Fields with a non-default initializer or constructor default, and empty strings, are always written, so configuration files get slightly larger. - **JSON decode:** a field missing from JSON decodes to the type default. - **XML decode:** a field missing from XML keeps its declared default, which older configuration files rely on. - **Why:** this fixes silent round-trip loss, for example `SamplingInterval = 0` read back as `-1`. - **Nested `[DataType]` types:** they are now generated inside their containing types. Without an explicit id, their default TypeId and XmlName are qualified (e.g. `s=Outer.Foo`); **wire identity change** for nested types. Colliding activator names get a `_2` suffix. - **Activators:** they are `internal` when the type, or any containing type, is not public (was CS0060). - **Matrix properties:** - `MatrixOf<T>` properties use the inline matrix encoding. - Matrices of non-sealed structures, or with `StructureHandling.ExtensionObject`, are written as ExtensionObjects. - Their published ValueRank is 2, because the rank can't be known from `MatrixOf<T>`. - **Published StructureDefinitions:** - They include base-class fields. - Nullable properties are no longer marked optional. - Inline structure fields name their concrete type. - **`[Flags]` enums:** they are published as plain enumerations with their member values, which matches how they are encoded. - **Other generated code:** - `Clone` deep-copies arrays and matrices of structures, Variants, ExtensionObjects and DataValues. - `IsEqual` and `GetHashCode` are consistent: EncodingMask, active union member and NaN handling agree. - Pooled `Reuse()` restores the freshly constructed state through the new `protected virtual ResetForReuse()`, and returns an instance to its runtime type's pool. - **Enum capture:** enum member values are captured culture-invariantly. Negative values were lost on cultures that use U+2212 as the minus sign. ### Model-driven generator: generated code, API and file names - **Removed API:** generated `*_Placeholder` proxy accessors (e.g. `PowerTrainTypeClient.GetMotorIdentifier_PlaceholderAsync`) and fluent `X_Placeholder()` accessors are removed; they could never resolve anything. - Proxy accessors now look children up by BrowseName with the right namespace. - `RoboticsClient` finds motors and gears by type and drives via `IsDrivenBy`. Its topology snapshots used to be empty. - **Renamed files:** generated files for a bound `[NodeManager]` are now named `{Namespace}.{Class}.NodeManager.g.cs` (also `.Factory` and `.FluentBuilders`). - **Generated NodeState code:** - Attributes are set through `baseState`/`nodeState` locals, so children named like base attributes (e.g. `DataType`, `EventNotifier`) compile. - Children that hide base members are declared `public new`, using names reflected from the runtime. - `global::` qualification is used throughout. - **Proxies and fluent builders:** a proxy whose supertype is excluded (e.g. Draft) derives from the nearest emitted ancestor. Fluent wrappers and NodeState factories fall back the same way; they used to fail with CS0246. - **Derived structures:** - The optional-field `EncodingMask` continues across inheritance. - A derived structure over a base type from another model no longer relies on `base.ResetForReuse`. - **Default values:** - NodeId, QualifiedName and ExpandedNodeId defaults resolve by namespace URI (`context.NamespaceUris.GetIndexOrAppend`) and server URI. - StatusCode, negative enum, `double`/`float` (shortest round-trip literal) and escaped string defaults now compile. - **Event records:** - Vendor event records have `Decoder.GetStandardFields(NamespaceTable)`, and their browse paths use the model's namespace. - A field whose namespace is unknown to the session no longer matches a namespace-0 field. - Companion `StandardFields` is `[Obsolete]`. - Vendor simple subtypes such as `Percent : Double` map to their built-in type. - **Other fixes:** - `MethodState.CallAsync` passes the real `objectId` to synchronous `OnCallMethod2` handlers; it used to pass `NodeId.Null`. - Enumeration fields with an explicit `Identifier` keep it, instead of taking a trailing `_<digits>`. - Explicit NumericIds are no longer reused by automatic numbering. - Dependencies are loaded in topological order. ### Generated address-space content - **Arguments:** - Methods without arguments no longer get empty `InputArguments`/`OutputArguments` properties (OPC 10000-3 5.7). - About 2,660 automatically assigned namespace-0 NodeIds of 15000 and above shift. Every NodeId in `StandardTypes.csv` is unchanged. - **Permissions:** type nodes and method declarations on types now carry the model's `RolePermissions`/`AccessRestrictions`. - **Restored nodes:** NodeSet import keeps nodes that were silently dropped before: - `NamespaceMetadata` objects and the NodeSet's own TypeDictionary variables; - ISA95 placeholders linked only non-hierarchically; - instances whose parent is in another namespace. - **Import details:** - ValueRank > 1 without ArrayDimensions keeps its rank. - `svr=` indexes map correctly. - Invalid MinimumSamplingInterval values import as -1, not 0. - Localized texts prefer the invariant or English translation. - **StructureDefinitions:** - They include inherited fields, including for subtypes without their own fields (e.g. `AnonymousIdentityToken` now publishes PolicyId). - The StructureType kind covers inherited AllowSubTypes fields. - Unions ignore IsOptional. - StructureFields never publish ValueRank 0; a OneOrMoreDimensions field without dimensions becomes rank 2. - Fields with ValueRank -2/-3 are published as scalar BaseDataType, which is what is on the wire. - **OptionSets:** - Implicit fields get the next free bit. - `OptionSetValues` uses null for undefined bits instead of "Reserved", and structure OptionSets publish all their bits (no 32/96-bit cap). - Numeric OptionSets are limited to their base type's width, and UInt64 bit 63 works. - **Schemas:** - Structures with inline-matrix fields are left out of the `.bsd` (5.2.5). - In the BSD, Variant-encoded and non-structure subtyped fields are described as `ua:Variant`. - In the `.xsd`, matrix fields are typed `ua:Matrix` with `Dimensions`/`Elements`, and `EncodingMask` is required. ### NodeSet / model input handling - **Model selection:** - The newest NodeSet of a model is chosen by `ModelVersion` (SemVer 2.0 precedence), then PublicationDate. The `Version` attribute, which Annex F says is not for comparison, is only a tie-break. - The order is total and does not depend on input order; referenced assemblies use the same rule. - **Default names:** - Invalid default Name/Prefix values are repaired (`201301ISA95` becomes `_201301ISA95`, `class` becomes `class_`). - Valid names are unchanged. - Sanitising no longer depends on the OS. - **`ModelUri` metadata** that doesn't match the NodeSet raises a warning instead of silently losing the model. - **Input detection:** - Only XML files whose root is `ModelDesign` or `UANodeSet` are treated as models. - A malformed NodeSet stays a NodeSet input, so it no longer breaks the ModelDesign pass. - **`IdentifierFile` paths:** they are normalized (`./`, `../`), and the suffix match requires a folder boundary. - **Dependency payloads** (`ModelDependencyV1`) carry field IsOptional, AllowSubTypes, ArrayDimensions and the producer's namespace tables in skippable trailers, so old and new readers stay compatible. ### Roslyn host, diagnostics, packaging - **New diagnostics:** - MODELGEN036: `[NodeManager]` target not supported (nested, generic, file-local or global namespace). - MODELGEN037: `[DataType]` target not supported (struct, generic, abstract, file-local or global namespace). - MODELGEN038: the definition of an encodeable base type can't be found. - **New validator errors:** - a structure with both optional and subtyped fields (OPC 10000-6 F.13); - multi-bit or duplicate OptionSet masks. - **Diagnostic locations:** `[DataType]`/`[NodeManager]` diagnostics point at real source locations again, so `#pragma` and `.editorconfig` apply. MODELGEN010 is the exception. - **Incremental caching:** pipeline values compare by value, so unrelated edits no longer regenerate the whole model or all `[DataType]`s. - **`ModelSourceGeneratorIgnore`** on a ModelDesign input is honoured; the design still resolves as a dependency. - **Cancellation:** a cancelled generation run propagates `OperationCanceledException` instead of reporting a MODELGEN003 error. - **Stack package:** the `Stack` NuGet package now ships `build/OPCFoundation.NetStandard.Opc.Ua.SourceGeneration.Stack.props`, so `StackSourceGeneratorMode` and `StackSourceGeneratorExclude` reach the compiler. ## Related Issues - No tracking issue. The audit, review and spec-grounding reports are available on request. Contains the matrix wire-format change discussed for D-5 (OPC 10000-6 5.2.5 Table 28). ## 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. (`docs/SourceGeneratedDataTypes.md`, `docs/NodeManagers.md`, generator readme) - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. (`UA.slnx` 0 warnings / 0 errors on net48 and net10.0) - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. The affected suites were run on net10.0: SourceGeneration Core/Roslyn/Stack, Types, Core.Encoders, Encoders.Fuzz, Client, Client.ComplexTypes, PubSub, Core, Server, GDS, Configuration, Di, Robotics, ISA95, Positioning, OpenUsd, XRegistry. net48 was built; its test legs, and the post-rebase verification, run in 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 Opus 5.5 <noreply@anthropic.com>	2026-09-29 00:47:40 +02:00
Marc Schier	c4e0721a29	Add a documentation learning path and fix issues found while verifying it (#4555) ... ## Summary ### Documentation - Stage the documentation index as a learning path: a first application, independent client, server, and PubSub paths, then data and services, companion models, production, advanced integration, and draft or experimental capabilities. - Add Getting started (repository samples or NuGet packages), OPC UA concepts, Your first information model, and a production readiness checklist. - Add Native AOT application publishing, a first PubSub publisher and subscriber exchange, and the UDP transport options. - Add or expand contents navigation throughout `docs/`, and keep contributor documentation separate. - Correct outdated SDK examples and capability guidance, consolidate repeated explanations into canonical guides, and repair local links and heading structure. - Rewrite What's New in 2.0 as a concise feature overview with short linked details; keep migration instructions in the Migration Guide. - Expand the feature catalogue with specification IDs, GDS, LDS, the OPC 10000-21 onboarding registrar, ISA-95, xRegistry, OpenUSD, and sample-local models; distinguish draft status, explain crypto pluggability, and bring performance guidance forward. - Address review feedback on wording, active monitored items during shadow reload, and the `Samples.md` filename and references. - Regenerate the `opcua-v20-migration` plugin's bundled copy of the migration guides so it matches the edited `docs/migrate/2.0.x` guides. - Merge the latest `master`, preserving upstream resource-isolation guidance within the task-based documentation index. - Retain deferred Lingity findings where appropriate; this PR does not claim that every document meets the Lingity score thresholds. ### Fixes found while verifying the guides Each fix has regression tests that fail without it. - **Client:** V2 subscriptions compared their lifetime with the operation timeout instead of the session timeout, and logged a spurious warning. `ISubscriptionEngineContext`, which is new in 2.0, gains a `SessionTimeout` property; implementations must add it. - **PubSub:** a subscriber now registers the `DataSetMetaData` configured on its readers (OPC 10000-14 6.2.9.4), and the UADP decoder decodes DataSetMessages that carry no ConfigurationVersion. Before, a secured subscriber of the reference sample received no DataSets. - **Source generator:** a ModelDesign method that declares its arguments inline, without a method type, produced code that did not compile; it now uses `MethodState`. The generated code of existing models is unchanged. - **MinimalClient sample:** waits for its first data-change notification before it disconnects. ### Known issues, not addressed here - UDP multicast: the publisher does not send on the interface that the subscriber joins, and `MulticastLoopback` defaults to false. The PubSub guide uses unicast for the exchange on one computer. - The UDP metadata announcement is sent unsecured on secured writer groups; a secured subscriber drops it with a warning. - A server does not start in globalization-invariant mode, because `ResourceManager` creates `CultureInfo("en-US")`. The Native AOT guide cautions against the mode. ## Validation - `git diff --check` and `./.azurepipelines/validate-migration-plugin.ps1` (15 bundled migration docs validated, plugin manifests synchronized). - Local Markdown links, anchors, file-link casing, and contents lists checked across all 87 documents under `docs/`. The index links every top-level guide and every target of the previous index. - Both Getting started routes, Your first information model, the PubSub exchange, and the Native AOT publish of the Getting started client and server were run as documented. The NuGet route and the tutorial use the public `2.0.0-preview.5` packages. - The changed libraries build for all target frameworks in Release without warnings. - Tests on .NET 10: Opc.Ua.SourceGeneration.Tests 199 passed; Opc.Ua.SourceGeneration.Core.Tests 3,977 passed, 8 skipped; Opc.Ua.Server.Tests `Fluent` category 621 passed; Opc.Ua.Client.Tests subscription-engine tests 70 passed; Opc.Ua.PubSub.Tests 1,374 passed, 2 skipped; PubSub fuzz replay 145 passed. The new and affected tests also pass on .NET Framework 4.8. - Earlier merge validation: 174 resource-isolation tests passed on .NET 10. - The full solution suite was not run locally. Review and validation artifacts are maintained outside the repository. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-09-28 19:19:39 +00:00
Cristi Pogacean	9b8a3b1188	Preserve node initialization and copied child ownership (#4544) ... # Description Preserve populated derived node children through optional initialization and give copied children independent ownership by their copied parent. - Track initialization from a source in `NodeState` and guard generated optional-default recreation without removing the optional initialization hook. - Reuse `CloneChild` for dynamic children, generated typed children, and method argument properties. Preserve the clone root's existing external parent. - Add cross-model initialization/copy regressions, method-argument and nested-child ownership tests, and inherited alarm-subtree checks. This PR contains only the six files for this fix group. Certificate-store, runtime-type-loader, publish-queue and transport-exception fixes are separate work. ## Related Issues Fixes #4525. Fixes #4526. ## Validation Fresh focused Release/net10.0 checks on the final base: | Fixture | Passed | Failed | Skipped | | --- | ---: | ---: | ---: | | CrossModelInstanceTypeDefinitionGeneratorTests | 9 | 0 | 0 | | Types NodeStateTests | 227 | 0 | 0 | | Core StateTypesTests | 13 | 0 | 0 | All three test commands exited 0. `git diff --check` and the changed-file diagnostic check are clean. Regression proof was also executed in the isolated worktree: with the three production changes reversed, six generator cases failed with the missing derived Temperature child while three controls passed; two copy cases failed parent-identity assertions while the external-parent/absent-arguments control passed. The production fix was restored and the focused checks are green. The retained red runs preceded the later upstream synchronization; the affected production paths were unchanged by that synchronization. Windows win-x64 NativeAOT publish and native execution also passed: main 178/178, Historian 2/2, MCP 3/3, no skips, every invocation exit 0. These native runs preceded the final CTT-configuration/notes-only fast-forward; this PR's six files were unchanged. Local validation is intentionally change-scoped for this work, with the full matrix and repository-wide checks left to PR CI. Repository guidance and test assertions/timeouts were not modified. Local commands used a NuGet v2 source override and disabled audit only on the command line; CI feed/audit policy is unchanged. Earlier broad attempts are not claimed as a completed local gate: an unmodified-base durable restart timeout was reproduced before synchronization (the later durable run passed), and the net48 runner stalled after recording passing client tests. A direct client retry subsequently exited 0, but the full net48 matrix was not completed. Those results are not substituted for the focused evidence above. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING doc. - [x] I have added regression tests and demonstrated failure without the production fix. - [x] The focused tests pass; no new compiler diagnostics were reported by those builds. - [ ] Coverage and the full net48/net10.0 matrix are verified by PR CI. - [ ] All CI, CodeQL, and review feedback is addressed. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-09-25 12:23:49 +00:00
romanett	96c5ef1623	Run the fluent configure pipeline in FluentNodeManagerBase (#4537) (#4538) ... ## Proposed changes Closes #4537. The configure pipeline now lives in `FluentNodeManagerBase`, so a hand-written fluent node manager only overrides `ConfigureAsync` and no longer writes `CreateAddressSpaceAsync` itself. `FluentNodeManagerBase.CreateAddressSpaceAsync` now: 1. loads the predefined nodes (as `AsyncCustomNodeManager` did before), 2. creates a builder for `NamespaceIndex` with `CreateFluentBuilder`, 3. awaits `ConfigureAsync(builder, ct)`, 4. runs `RegisterAuthoredNodesAsync` and then `CompleteConfigureAsync`, 5. finishes with `SealConfigurationAsync`, which seals the builder, replays `NotifyNodeAdded`, then activates behaviors and simulations. ### Breaking change (2.0) Some managers need a different order, for example a post-setup runner before sealing, a builder for another namespace, or work after sealing. They must stop calling `base.CreateAddressSpaceAsync`, because that would now run a second, empty pipeline and seal the manager-owned registries too early. These managers now call `LoadPredefinedNodesAsync(SystemContext, externalReferences, ct)` and keep driving their own pipeline: - the source-generated `NodeManager` template (the ordering test in `NodeManagerGeneratorTests` is updated) - `DiNodeManager`, `Isa95NodeManager`, `PositioningNodeManager`, `RobotIntentNodeManager`, `VisionNodeManager` - `samples/OpenUsd/SiteCompositionServer/SiteNodeManager` These now use the base pipeline: - `samples/FluentApi/BasicFluentApiServer/BasicFluentNodeManager` becomes a single `ConfigureAsync` override. Before this change it never sealed its builder. - the hosting `FluentNodeManager` (from `AddNodeManager(namespaceUri, build)`) becomes a `ConfigureAsync` override. It now seals through `SealConfigurationAsync`, so `OnNodeAdded` handlers wired in the build delegate fire. Before, it called `builder.SealAsync()` directly and they did not. Docs (`NodeManagers.md`, `DeviceIntegration.md`) are updated. `NodeManagers.md` gets a short hand-written `ConfigureAsync` example. ## Types of changes - [x] Breaking change (fix or feature that would cause existing functionality to change) - [x] Enhancement (non-breaking change which adds functionality) ## Checklist - [x] I have read the CONTRIBUTING doc. - [x] I have signed the CLA. - [x] I ran tests locally with my changes, all passed. - [x] I fixed all failing tests in the CI pipelines. - [x] I fixed all introduced issues with CodeQL and LGTM. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added necessary documentation (if appropriate). ## Tests New `FluentNodeManagerBasePipelineTests`: nodes authored in `ConfigureAsync` get registered, the Objects-folder reference is mirrored into `externalReferences`, `OnNodeAdded` fires once, and the builder is sealed afterwards. Local run on net10.0, all passing: - Opc.Ua.Server.Tests (Fluent/Hosting/StateMachine/RuntimeNodeSet filter): 1286 - Opc.Ua.SourceGeneration.Core.Tests (NodeManager filter): 40 - Opc.Ua.Di.Tests 426, ISA95 136, Positioning 69, Robotics 636, Robotics.Intent 22, Vision 484 (+1 skipped), Vision.Intent 14, OpenUsd 921 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-09-25 07:48:29 +02:00
Marc Schier	138086863b	Fix audit findings and review regressions across the OPC UA stack (#4503) ... ## Scope Audit fixes and review follow-ups across the OPC UA Core, Client, Server, transport, and redundancy components. This includes implemented Tier 1, Tier 2, and Tier 3 work; it does not claim that every item in the 244-finding audit backlog is complete. The review batch fixes mandatory full-key record protection and owned plaintext buffers; transactional pending-certificate claims; shared-channel nonce and certificate ownership; discovery and token-policy validation; reconnect/callback ordering without changing unbounded buffering defaults; server permissions, filters, monitored-item lifetime, and alarm topology; historian paging, retention, and annotations; atomic user/password metadata; and canonical file identities with bounded alias and role refresh workers. ## Review-batch verification at `ab686ccae` The following results come from fresh test builds using `dotnet test`, `--no-restore`, `-m:1`, and an explicit `CustomTestTarget`. They replace the earlier description's library-build/`--no-build` totals, which did not establish the state of the merged test sources. | Scope | Target | Passed | Skipped | |---|---|---:|---:| | Core review regressions | net10.0 | 827 | 0 | | Client and lifecycle regressions | net10.0 | 630 | 0 | | Server review regressions | net10.0 | 1970 | 0 | | Redundancy Server / Client / PubSub | net10.0 | 276 / 10 / 11 | 0 | | Sessions / file-and-alias / streaming / HTTPS-WSS integration | net10.0 | 8 / 21 / 30 / 20 | 0 | | Core review regressions | net48 | 786 | 39 | | Client recovery and policy regressions | net48 | 54 | 0 | | Server ownership, aliases, queues, and user-state regressions | net48 | 137 | 0 | Every listed run exited successfully. The 39 net48 skips are existing platform/capability cases. New key-agreement tests assert unavailable-policy behavior on unsupported targets rather than adding skips or pretending those targets performed an unsupported handshake. Additional exact node/permission/filter/alarm checks passed 13 Core and 46 Server cases. The PR-scoped formatter verification covers 26 projects and 389 non-generated C# files. A Roslyn syntax audit covers 53 changed interfaces and 263 members, with no missing parameter/type-parameter/return contracts and a 120-column limit for their declarations and documentation. No full-solution, all-target-framework, or NativeAOT matrix is claimed from these local runs; the new CI run must establish its own result. ## CI follow-up at `e1bc3ac3b` The follow-up fixes the failures reported by Azure build 18400. TLS certificates retain configured DNS names, including mixed-case URI schemes, without relaxing hostname validation. Reverse-WSS fixtures use the DNS identity in the client certificate. Reconnect completion is published only after cycle cleanup, so a subsequent reconnect cannot join an old completed cycle and retain old certificates. Certificate-alarm roots now support event subscriptions. The pump sample has one continuous notifier path, with no duplicate shortcut. Historian and aggregate fixtures share the provider's retention clock. Alarm collection lets the server expire idle Publish requests rather than canceling only the client wait. The corrected server, WoT, and managed-session fixtures exercise valid requests and initialized lifecycle contracts. Downlevel crypto tests preserve real credential and key-ownership assertions without enabling unsupported raw-ECDH policies or adding skips. Fresh local evidence after integration includes: | Scope | Target | Passed | Skipped | |---|---|---:|---:| | Client lifecycle / channel ownership / builder fixtures | net10.0 and net48, each | 96 / 103 / 123 | 0 | | Combined server lifecycle, events, aggregates, crypto, and contracts | net48 | 358 | 0 | | Full lifecycle plus certificate-alarm and aggregate fixtures | net10.0 | 116 | 0 | | Final server-contract and history-routing delta | net10.0 and net48, each | 84 | 0 | | Subscription / XRegistry / WoT event regressions | net10.0 | 7 / 2 / 3 | 0 | | Full Sessions project | net10.0 | 780 | 379 | | Full ComplexTypes project / downlevel resolver fixtures | net10.0 / net48 | 2854 / 15 | 0 | | Full WoT project / downlevel structured-write fixtures | net10.0 / net48 | 1496 / 27 | 0 | | TLS domain-name regressions | net10.0 and net48, each | 21 | 0 | All listed runs exited 0. The Sessions skips come from existing endpoint and policy capability guards; they are not counted as passes. No skip was added. Worker validation also covered full DI (426 passes), full History (540 passes, 21 existing skips), 38 exact downlevel event/history failures, and 60 repeated certificate-rotation race cases. The final crypto fixtures passed 73 cases on each target with the unchanged leak detector; a temporary premature-disposal mutation made all three borrower tests fail and was then removed. All 29 C# files in this follow-up pass included-file formatting at info severity across 12 projects. The complete PR interface audit now covers 54 interfaces and 271 members with zero missing contracts or line-width findings. No full-solution or NativeAOT matrix is claimed. New-head CI has started and remains authoritative for the remote result. ## Test-result gate follow-up at `4b6e1203a` Azure build 18401 reported the net48 Client job as successful despite a test-host crash after only 264 passing tests. Its old evaluator counted per-test failures but ignored the failed run verdict. This was a partial run, not a benign exit after the suite had completed. The shared result evaluator now rejects aborted, errored, incomplete, malformed, or inconsistent TRX reports. Each file must describe a completed run, so a passing file cannot mask another file's failure. It retains support for completed runs followed by a benign nonzero host exit, existing skipped-result conventions, and no-TRX MTP jobs. All 78 regression cases pass after integration using Windows PowerShell 5.1. Real completed Sessions and Client reports also pass. The process-test fixture follows the existing NET10_0-only tools-test pattern; its net48 build is not counted as a test run. Formatting passes for the added test file. The Client host crash itself has not reproduced locally. The isolated fixture passed 59 tests, and two fresh full fast-CI selections each completed with 2,392 passes and three existing skips, including a run with crash collection enabled. The CI merge tree is identical to the tested source tree. No speculative runtime change was made, and the original partial CI run is not claimed as a full-suite pass. Azure build 18402 has now completed the full net48 Client run: 2,392 passed, three existing console skips, and a successful test-host exit. The corrected evaluator independently validated outcome=Completed. Across the matrix, 119 test-project jobs succeeded; Sessions net10.0 was canceled when the remote provider deprovisioned its runner, so the overall Azure pipeline was canceled. That cancellation is not claimed as a code failure or a successful run. CodeQL and three Docker image jobs were still running at the latest check. The migration discovery failure remains separate. ## Documentation follow-up at `57ed02bea` The environment blocker has been removed by the maintainer. The three bundled migration references now exactly match the canonical documents after deterministic link rewriting; all 15 bundled documents pass `validate-migration-plugin.ps1`. The canonical migration prose was not rewritten. `FileSystemClient.md` now documents that extended Windows roots preserve literal trailing-dot names, completing the filesystem review follow-up. The filesystem thread has been replied to and resolved after publication. The review pass now has 108 of 110 threads resolved. The two explanation-only threads remain open as requested; no unfinished change request is being represented as resolved. ## Cross-platform follow-up at `0d9d936a8` With migration discovery restored, the Linux and macOS test matrix exposed two stale durable-subscription expectations. The expected value 83,442 encoded the old 32-bit overflow when converting 3,600 hours to milliseconds. The existing overflow-safe server correctly returns 14,400,000 lifetime intervals at a 900 ms publishing interval. Only those two expected constants changed; production behavior, negotiated-hour checks, and all other assertions remain unchanged. The exact local lifetime selection reproduced both failures before the correction. The full durable suite now passes 36 cases with 11 existing skips on each of net10.0 and net48. All four lifetime cases executed and passed on both targets, and included-file info-severity formatting passes. ## Session shutdown follow-up at `8daf7f091` A late net48 Sessions failure exposed a shutdown race. Canceling an in-flight keepalive read can produce a transport `ServiceResultException`. The retiring worker treated its own cancellation as proof that the peer was unavailable, so `CloseAsync` or `DisposeAsync` could skip the server-side `CloseSession` request. The worker now ignores that service error only when its own token is canceled; genuine peer errors still update liveness. The full integration fixture reproduced the original error-code mismatch. Four deterministic mock-transport cases then produced three failures and one passing genuine-error control before the fix. They verify successful close, returned server error, asynchronous disposal, and unchanged behavior for a real keepalive failure. Fresh related runs pass 75 client and 101 integration cases on each of net48 and net10.0, with no skips. Ten repetitions of the formerly flaky full net48 fixture pass all 1,010 cases. All three changed files pass info-severity formatting, and temporary diagnostic instrumentation was removed. ## Atomic listener snapshot follow-up at `82dc22b5d` Ubuntu Core CI exposed an unsafe formatter rewrite in TCP listener disposal: `ConcurrentDictionary.ToArray()` had become a collection spread. A concurrent channel removal between the spread's count and copy operations could leave a default entry, causing a null dereference during disposal. The listener now uses the dictionary's atomic `Values` snapshot and disposes those channels outside the listener lock. Deterministic regressions close zero, one, or all channels at the copy boundary. Before the fix, the one/all-removal cases reproduced the exact null dereference; the unchanged-count control passed. All 23 admission/listener cases now pass on net10.0 and net48, and the full fast Core net10.0 suite passes 5,267 cases with 88 existing platform skips. Both changed files pass info-severity formatting. A bounded audit of the formatter's other snapshot rewrites found no further verified unsafe concurrent-registry copies. ## Shutdown-test observation follow-up at `58403ee8b` The net48 shutdown-order test raced its own observation: it read `m_publishWorkerTask` after starting shutdown, which can legitimately drain the worker and clear that field immediately. The test now captures the worker before shutdown starts and confirms it is initially running. It still requires the worker to finish while shutdown remains blocked by the held manager semaphore. Production behavior is unchanged. A temporary scheduling perturbation reproduced the exact null-field failure. The corrected capture passed 100 perturbed iterations; both the temporary repeat and yield were then removed. The final 126 subscription/lifecycle cases pass on each of net48 and net10.0, and formatting passes. A complete local Sessions net10.0 run also passed 780 cases with 379 existing capability skips in 7m20s after the earlier keepalive correction. ## Retirement and alias-startup follow-up at `1a048ff39` The shadow-generation test now observes its documented asynchronous cleanup through the existing bounded helper. Removing the replacement does not join a background drain that already claimed the retired generation. The test still checks live-registration removal immediately and requires the old node to disappear; no timeout or production lifecycle behavior changed. All 81 lifecycle cases pass on each of net10.0 and net48. Initial alias-node materialization now retries one timed-out, read-only snapshot query before creating nodes for that root. The retry is logged through source-generated logging. Each query retains the original shared 100 ms deadline; repeated timeouts, other errors, and cancellation still abort startup. Client searches and live refresh do not gain this immediate retry. Fault-injection tests cover success after one timeout, a persistent timeout, a different store failure, and cancellation. All 187 related alias/hosting cases and the original certificate-rotation integration case pass on each target; the positive test records exactly one retry warning. The alias documentation now accurately describes the shared request budget. All changed C# files pass formatter checks, and the complete 54-interface/271-member documentation audit passes. Fresh CI is running on `1a048ff39`; the previous completed matrix exposed only the two failures corrected in this batch and their dependent summaries. ## Sample endpoint allocation follow-up at `d08a94925` The strong-cluster test harness allocated OPC UA and Raft endpoints in separate released-port batches. Uniqueness was enforced only inside each batch, so the same OS-assigned port could appear in both protocol groups. The harness now requests one distinct batch for both groups and splits it before launching any replicas. Deterministic one- and three-replica allocation cases reproduced overlapping groups with the old plan and pass with the corrected plan. The complete 17-case short-haul sample suite passes without skips; all failover, historian-continuation, and service assertions remain unchanged. Both changed files pass info-severity formatting. This prevents intra-cluster self-collisions but does not claim to reserve released ports against unrelated external processes. ## Code-review follow-up at `085fd9636` A review of the complete branch diff found five defects this PR introduced. All are now fixed, each in its own commit. `RelatedTo`'s recursive-to-iterative rewrite defaulted the optional `includeTypeDefinitionSubtypes` and `includeReferenceTypeSubtypes` operands to `false`. Part 4 §7.7.4 specifies both default to `true`, so a filter that omitted them silently stopped matching subtypes. Dynamically created key-credential nodes derived their namespace from the containing folder, which is the standard `i=18155` node in namespace 0 — the namespace reserved for the OPC UA standard address space; they now resolve the server-owned namespace and fail with `BadInternalError` rather than guessing index 1. The retained audience-less `CreateTokenData` overload produced tokens that verification always rejects, so it is now `[Obsolete]` and throws instead. A failed reference to a remote parent left the new node registered with a one-sided inverse reference, and is now rolled back. An existing `.pfx` is restricted before it is rewritten, rather than after its new key bytes are on disk. One further reported finding was rejected after investigation. `JwksIssuerKeyResolver` deliberately does not dispose retired key snapshots: `RetiredKeysAreCollectibleWhileResolverRemainsAliveAsync` proves the resolver retains no retired snapshot, and `ReturnedKeysSurviveRepeatedRefreshAndResolverDisposalAsync` requires already-returned keys to keep verifying across refreshes and resolver disposal. Adding disposal would have broken that tested contract, so the exploratory change was reverted. Validated on net10.0 and net48: 256 Core and 633 Server cases pass, with one pre-existing net48 skip. The build reports zero compiler and analyzer diagnostics, all eight included-file info-format checks pass, the contract audit covers 58 interfaces and 310 members with no findings, and the 15 bundled migration documents validate. All 122 review threads are resolved; the key-credential namespace thread was reopened to retract an earlier incorrect answer and then resolved against the real fix. ## Review-body follow-up at `da0d59cfb` The re-review at `8daf7f091 + 82dc22b5d` listed four items in its summary that had no inline thread of their own, so they had not been picked up. All four are now fixed. Both event-dispatch permission gates had been tightened from not-Bad to `IsGood`, which silently dropped events whose permission verdict is Uncertain; the data-change path in the same file still treated Uncertain as deliverable, so the two had diverged. `EventSourceRegistry` only cleared `ConsecutiveFailures` when a source was deactivated, so a source that had backed off to the 30-second cap kept that delay for the rest of the process even after starting successfully. `UserNameIdentityTokenHandler` built a temporary issuer chain whose `Add` calls take independent certificate references, and never released them. The fourth was the receiver-side sequence rollover, which the reviewer had correctly re-flagged as still open despite its thread being resolved. `m_sequenceRollover` was set permanently, so the second legal wrap on a long-lived channel was rejected and faulted the channel along with the transport-reconnect OPN. OPC 10000-6 does not cap a SecureChannel at one wrap. The flag is now cleared once an accepted number has traversed the normal range, which keeps rejecting a replayed low number that never left the rollover window. Validated on net10.0 and net48: 636 / 593 Core cases and 135 / 134 Server cases pass, including the new `UaSCBinaryChannelSequenceRolloverTests` and the Uncertain-permission delivery cases. The build reports zero compiler and analyzer diagnostics and the changed files pass included-file info-format checks on both targets. The previous head `085fd9636` was fully green: 279 checks passed and Azure build 18414 succeeded with 137 of 137 jobs, after the platform automatically retried a job whose NuGet tool download had hit a transient TLS error. ## Reported issues #4508 and #4509 at `6ad1ecf16` **#4508 (V2 subscription recreate deadlocks against a faulted transport) was not yet fixed and is now closed by `bac260a06`.** Both drain sites the report identifies were still unbounded. A publish worker that started its request before the quiesce parks inside the service call on the channel ready gate while holding an active publish request; the recreate that would make the channel ready is the very operation awaiting the drain, so the wait never ends. The drain now cancels in-flight attempts through a per-attempt token linked to the worker token. An aborted worker rolls its acknowledgements back, releases the request and returns to the paused park instead of terminating, so it resumes when the caller leaves the quiesce. Publish ingress stays quiesced for the caller's operation, so `DropPendingForSubscription` keeps the guarantee the report asks to preserve and no blind timeout is added. The new regression parks a worker on a cancellable gate that never completes on its own: it reproduces the hang for the full 30-second budget before the change and passes after it. **#4509 (identity provider fails the initial connect) was already fixed for its primary symptom.** `HandleConnectAsync` materializes the provider's identity through `CreateIdentitySelectionContext` before the first `CreateAsync`, so Anonymous is no longer substituted against a UserName-only endpoint. `6ad1ecf16` adds the two remaining details from the report: the enabled-policy set now includes the negotiated channel policy, so a client cannot reject the policy it already accepted as `NotEnabledByClient`; and when discovery has not populated the endpoint's token policies the connect keeps the configured identity so the attempt fails at the transport layer for the reconnect policy to retry, instead of failing at the identity layer. Validated on net10.0 and net48: 477 / 473 client session and subscription cases pass, the build is free of compiler and analyzer diagnostics, and the changed files pass included-file info-format checks on both targets. The full `Opc.Ua.Sessions.Tests` suite also passes locally on net10.0 with 782 passed and 379 pre-existing capability skips. GitHub checks are green on this head: Build and Test .NET 10.0, Docker Sample Images CI, and NuGet Publish all succeeded. Azure build 18417 completed 134 of its 135 jobs successfully; its net10.0 Sessions job was cancelled by the hosted-agent provider ("Received request to deprovision"), the same infrastructure flake that hit build 18409 earlier on a head predating these changes. That job is not claimed as passing, and the local full-suite run above covers the same project. The previous head `da0d59cfb` completed Azure build 18415 with every job succeeded. ## Review follow-up at `a08f0c12b` Eleven new review threads arrived, nearly all documentation. Before acting on each "this is new in 2.0" remark I checked the API against `master378` rather than assuming, because a genuine 1.5 API still needs its migration entry. `ManagedSession`'s server-redundancy refresh had two hard-coded two-second bounds. The new `ServerRedundancyOptions` exposes `RefreshTimeout` for the metadata read and `PeerDiscoveryTimeout` for each peer lookup, defaults both to the previous two seconds, validates that a bound is positive or `Timeout.InfiniteTimeSpan`, and flows through `ManagedSessionOptions`, `ManagedSession.CreateAsync`, the fluent builder, and `DefaultServerRedundancyHandler`. `AsyncCustomNodeManager` and `KeyCredentialBridgeAuthenticator` are both absent from `master378`, so their obsolete members had no compatibility constraint and are removed rather than carried: `AsyncCustomNodeManager.ReadInitialValue`, the audience-less `CreateTokenData` that only threw, and the audience-less `CreateProof` that produced a proof no server accepts. `CustomNodeManager2.ReadInitialValue` and the `MonitoredNode2` migration section are genuine 1.5 APIs and are untouched. The documentation now states master's behaviour instead of the delta from 1.5.378, with each moved item already covered under `docs/migrate/2.0.x/`. `IHistorianAnnotationProvider` is described by its annotation-time key rather than as "legacy" — it is new in 2.0, so the label implied a compatibility constraint that does not exist. The KeyCredential bridge payload's `"version"` field is explained as a vendor-extension format version with no OPC UA counterpart, and its version-1 upgrade narrative is gone. `CertificateManager.md`'s migration walkthrough is replaced by the resolver API it targets. `NodeManagers.md`, `Sessions.md`, `AsyncServerSupport.md`, `Certificates.md`, and `Profiles.md` no longer announce features relative to 1.5. `Benchmarks.md`, `WhatsNewIn2.0.md`, `DeveloperGuide.md`, and `docs/migrate/` keep their 1.5.378 references, which are their subject. CI then exposed a defect this batch introduced: five fixtures resolve `ManagedSession`'s private constructor by its exact positional parameter types, so the added parameter compiled cleanly but made `GetConstructor` return `null` at run time. `7df1d5c40` updates every site; `Redundancy.Client` passes 126 of 126 and the affected client fixtures 112 of 112. The BouncyCastle question is answered and tracked in #4510: `Opc.Ua.Core` already project-references `Opc.Ua.Security.Certificates`, which already carries the conditional BouncyCastle reference, so the raw-ECDH polyfill belongs there rather than as a new Core dependency. The `IUserDatabase` metadata question is addressed to another maintainer and is deliberately left open. ## Reported issue #4509 follow-up at `5d4d13d63` The reporter verified `6ad1ecf16` and found one case still failing, correctly. The empty-token-policies branch kept the configured identity so a server that is down surfaces a transport failure rather than an identity error. A provider-only configuration has no configured identity, so it kept `null`: `OpenCoreAsync` turned that into Anonymous and `OpenValidateIdentity` rejected it with `BadIdentityTokenInvalid`, silently ignoring the configured provider. The branch now requires a configured identity to defer to. Without one the provider is consulted, so its own `BadIdentityTokenRejected` names the offered and rejected policy set instead. Two regression tests cover both configurations; the first fails against the previous behaviour by reaching the session factory with a `null` identity, which is the exact step that produced the reported error. That branch had shipped without a test, which is why the case was missed. ## Review and CI status at `5d4d13d63` **All actionable review threads are resolved: 132 of 133.** The single open thread is a question the reviewer addressed to another maintainer about moving user metadata into `IUserDatabase`; it is left open for them rather than resolved silently. Every resolved thread carries a reply naming its fix commit. **Latest-head CI is green on `5d4d13d63`: 279 checks passed, zero failed, zero pending**, with one conditional signed-package publishing job skipped. Local validation for this batch: the full `Opc.Ua.Client.Tests` suite passes 2,430 of 2,430 on net10.0, `Opc.Ua.Redundancy.Client.Tests` 126 of 126, and `Opc.Ua.Server.Tests` 590 of 590 for the affected selections. Both net10.0 and net48 build clean for `Opc.Ua.Client`, `Opc.Ua.Server`, and the changed test projects, and all changed files pass included-file info-severity formatting on both targets. The PR remains open; no merge was performed. As stated at the top, this does not claim that every item in the 244-finding audit backlog is complete. ## Latest CI follow-up at `b6f2dfa43` Azure build 18442 exposed a net48 compile regression in the Kafka adapter: `RuntimeHelpers.GetUninitializedObject` is unavailable on .NET Framework. Commit `2cf1e0eed` now uses normal `ProducerMessage` construction on `NETFRAMEWORK` while retaining reflection-based construction for modern targets that can load a different Dekaf runtime asset. The exact net48 CI selection passes 92 of 92 tests, and the netstandard2.1 adapter guard selection passes 11 of 11. After merging the latest `master`, GitHub Actions run 35643592139 is green. The initial Ubuntu Core.Schema job built successfully and passed 160 of 160 tests, then hit an external 403 while finalizing its test-result artifact; its failed-job rerun completed successfully. Azure build 18451 was canceled when its remote provider deprovisioned the net10.0 Sessions runner (`Received request to deprovision: The request was cancelled by the remote provider`). Azure attached no test task log or source error, and the canceled parent left that child check stale as in-progress. No source workaround, test skip, or check relaxation was made for either infrastructure failure. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68f26810-22b8-476f-abcd-0a8c82d6e208	2026-09-22 09:10:58 +02:00
Marc Schier	331a71ac2c	Fix AI model version and inference argument handling (#4512) ... ## Summary - Derive the AI server''s advertised specification version from generated NodeSet metadata (new source-generated `ModelVersions.Target` constant), so `AINodeManager.SpecificationVersion` can no longer drift from the shipped model. - Preserve inference parameters through inline, asynchronous, fallback, and oversized-transfer execution instead of silently discarding them. - Reject unsupported `PayloadUri` input explicitly with `BadNotSupported` before any backend call, job creation, or transfer allocation. - Apply the supported `temperature` / `max_tokens` / `top_p` parameters in both built-in backends via a shared parser. Fixes #4505 Fixes #4506 ## Review follow-ups (f8754f618) - REST backend: a JSON body field is dropped only when its matching call parameter was actually supplied, so an explicit `temperature` no longer erases payload-supplied `max_tokens` / `top_p`. - Call parameters accept every built-in integer encoding (`Int64`, `UInt64`, `Int16`, `UInt16`, `Byte`, `SByte`) and booleans, instead of failing the whole `Invoke` when a client encodes `max_tokens` as `Int64`. - A null `String` Variant is rejected rather than inserted into the non-nullable backend parameter dictionary. - The invariant-formatting regression now runs under a comma-decimal culture so it genuinely discriminates. ## Testing - `dotnet test tests\Opc.Ua.AI.Tests\Opc.Ua.AI.Tests.csproj -f net10.0 --no-restore` — 186 passed, 0 failed - `dotnet test tests\Opc.Ua.SourceGeneration.Core.Tests\Opc.Ua.SourceGeneration.Core.Tests.csproj -f net10.0 --no-restore` — 3962 passed, 8 skipped - `dotnet format whitespace --verify-no-changes` clean on the changed files; build emits no warnings.	2026-09-21 17:35:46 +00:00
Copilot	02fb72e2d3	Support guid/opaque nodeids in NodeSet import (#4507) ... # Description `NodeSetToModelDesign.ImportNode` only stored Numeric and String NodeId identifiers. Guid and Opaque identifiers (both valid per OPC 10000-3 5.2.2) fell through to an `Information` log line, so the node was registered in `NodesByQName`/`NodesById` with no identifier at all and code generation continued against a node whose identity had been discarded. The `ModelDesign` schema has no representation for Guid/Opaque identifiers (`NodeDesign` exposes only `NumericId`/`StringId`), so the import now fails loudly instead of emitting output that looks correct. - **Importer**: the fall-through branches in `ImportNode` are replaced with an `InvalidDataException` naming the NodeId, browse name and identifier type. This matches the existing unsupported-input handling in the file and surfaces through `ModelCompilation` as a generator diagnostic. - **Logger removal**: `m_logger` had no remaining use in `NodeSetToModelDesign` and was removed along with its `Microsoft.Extensions.Logging` import. - **Tests**: `ImportUnsupportedNodeIdTypeThrowsInvalidDataException` covers Guid, empty Guid and Opaque identifiers via an in-memory NodeSet. - **Docs**: the source generator readme states the NodeSet2 identifier restriction. ```csharp else { // The model design can only carry numeric and string identifiers. // Guid and Opaque identifiers (OPC 10000-3 5.2.2) would be dropped // and code would be generated for a node without any identity, so // the import fails instead of silently producing wrong output. throw new InvalidDataException( $"NodeId ({input.NodeId}) of node '{input.BrowseName}' uses the " + $"{nodeId.IdType} identifier type which is not supported by the model " + "design. Only Numeric and String identifiers can be imported."); } ``` Note on behaviour: a null/unspecified NodeId parses as a Numeric identifier (`i=0`) and is still handled by the numeric branch, so the removed `IsNull` branch was only ever reachable for an empty Guid or Opaque identifier, which is now reported as unsupported. ## Related Issues ## 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. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #4504 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-09-21 14:52:55 +02:00
Marc Schier	9f14ddf5de	Preserve non-child hierarchical references during NodeSet import (#4491) ... # Description Preserve forward and inverse same-namespace hierarchical references that are not already represented by the NodeSet importer's parent/child relationship. A reference is implicit only when both its endpoints and its exact reference type match the selected ownership relationship; additional links do not reparent their targets. Also fix the generated-reference duplication that blocked the previous attempt in #4473. Inherited and declared `NodeDesign` objects can differ structurally while resolving to the same runtime reference. Deduplicating by the emitted reference-type ID, direction, and target ID prevents identical `AddReference` calls from throwing during Robotics/OpenUSD startup. - Add 21 importer/generator regression cases, including a minimized recursive-placeholder reproduction of the startup failure. - Strengthen the reference-server test to check the original missing forward links, inverse counterparts, subtype filtering, and unchanged ownership. - Document ownership versus graph references in `docs/NodeManagers.md`. ## Validation Confirmed the missing-reference cases fail before the importer fix, and the minimized duplicate-reference case fails before the generator fix. The original Robotics/OpenUSD fixture passed before the changes, failed with the importer-only candidate, and passes with both fixes. | Suite | net10.0 | net48 | | --- | --- | --- | | SourceGeneration.Core.Tests | 3,961 passed; 8 existing skips | 3,961 passed; 8 existing skips | | SourceGeneration.Stack.Tests | 94 passed | 91 passed | | SourceGeneration.Tests | 185 passed | 185 passed | | Di.Tests, including all 17 RobotOpenUsdE2eTests | 425 passed | 425 passed | | ReferenceServerTests | 45 passed | 45 passed | Executed using `CustomTestTarget` with `dotnet test tools/SourceGeneration.slnx`, the Di test project, and the `ReferenceServerTests` filter in the Server test project. Scoped whitespace/style checks and `git diff --check` also passed for the changed code. Pre-existing analyzer diagnostics outside the changes were left untouched. The full `UA.slnx` test suite has not been run locally. CI and CodeQL results remain pending. ## Related Issues - Fixes #4484. - Addresses the hierarchical-reference limitation deliberately deferred in #4473. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received.	2026-09-15 13:06:47 +00:00
romanett	e639932b95	Fix source generator defects from the Client/Types/SourceGeneration audit (#4473) ... # Description Fixes the SourceGeneration findings of a read-only bug audit of `a05a36055` — sections 15 (host generators, Roslyn side), 16 (SourceGeneration.Core schema / ModelDesign loader) and 17 (SourceGeneration.Core generators). 36 of the 39 findings are fixed; each has a regression test. ### Generated-code correctness - **State machine NodeId constants** were emitted as `const uint` even when the aliased `Objects` constant is a `string`, so every model using string NodeIds with a `FiniteStateMachineType` subtype failed with CS0029. The constant type now follows the aliased node, looked up from the same source `NodeIdGenerator` emits from — top-level nodes **plus each node's instance hierarchy**, since a state or transition only ever exists as a hierarchy entry. - `NodeIdGenerator` emitted string identifiers into C# string literals without escaping them. - Structure field names are authored data, so they can be C# keywords or collide with the members every generated data type carries. Properties and `{Type}Fields` members now go through a sanitizing, collision-breaking mapping (including two siblings that sanitize to the same identifier). **Wire names are untouched** — OPC 10000-3 §8.51 requires the StructureField name to match the BrowseName, and the `DataMember`/encode/decode names still come from `Parameter.Name`. - `NodeStateGenerator`'s copy-on-write cast now uses the fully qualified data type name, which a cross-model structure needs. - `GetScalarValueAsCode` emitted `(double)NaN` and the infinities, which do not compile. - `TemplateWriter` formatted with the current culture, so a machine with a comma decimal separator emitted invalid generated source. ### Served schema documents - The binary schema described a **different wire layout than the generated `Encode`/`Decode`**: unions carried no `SwitchField`/`SwitchValue`, and structures with optional fields carried no encoding-mask presence bits. Both are now emitted per **OPC 10000-6 §5.2.7**, matching the layout the runtime `BsdSchemaGenerator` produces. A structure with more than 32 optional fields now fails generation instead of silently wrapping `1 << 32` back onto the first field's bit. - Documentation text and attribute values are XML-escaped. ### ModelDesign loading and validation - Identifiers pinned in a CSV were dropped whenever any other loaded namespace declared the same symbolic name. Removal is now scoped to the file's target namespace, and explicit `NumericId`s are keyed by `(namespace, id)` so the same number in two namespaces is legal. - Declaration order inside a design file no longer matters — base types are imported and validated ahead of their subtypes. - OptionSet `OptionSetValues` were truncated at bit 31 (signed shift) and bits 32–63 of a 64-bit option set were never emitted. - An authored `ValueRank` is no longer overwritten by the shape of the default value (OPC 10000-3 §6.2.8: a ValueRank may only be further restricted), and a VariableType data type may be narrowed any number of levels deep rather than exactly one. - NodeSet import: `AccessRestrictions="0"` means "no restrictions", not `EncryptionRequired` (OPC 10000-3 §8.56) — an unrecognised mask stays **fail-closed**; a NodeSet without `<Models>` reports what is wrong; an optional `<References>` no longer throws; the parent/child reference scan looks at the parent's references; and `IsTypeOf` terminates on the self-referential root reference types NodeSets routinely declare. - Version comparison is numeric per component (ordinally `"1.05.9"` sorts above `"1.05.10"`) and now lives on `SemVer`, so the NodeSet collection and the dependency tie-break cannot pick different winners; a publication-date version no longer outranks a real one. ### Host generator - `[DataType]` on an `enum` is matched again (an enum is not a `TypeDeclarationSyntax`), MODELGEN003 renders its arguments instead of the raw template, and the Types hint name keeps the namespace's dots so `A.BC` and `AB.C` cannot claim the same one. - Compilation-derived pipeline stages carry value comparers, so editing a file no longer re-runs the entire model generation on every keystroke. - A NodeSet that fails to load skips that file rather than every remaining one; a file listed twice as an `AdditionalFile` is deduped; superseded NodeSet versions are still recognised as NodeSet inputs and not generated a second time by the ModelDesign pass. - `LogLevel.Critical` maps to a diagnostic instead of being silently dropped. ### Client - `EventRecordFieldReaders` gained the readers the event record generator needs (including `Variant`, enumerations and the array forms). Fields whose type had no reader were declared on the record but never populated, so they read as `null`/`default` for every event. ## Not fixed (deliberately) - **Forward same-namespace hierarchical references in `NodeSetToModelDesign.ImportReferences`** — the audit flags the inverse-only handling as asymmetric. Making it symmetric compiles and passes every generator unit test, but regresses the address space: all 17 `RobotOpenUsdE2eTests` fail in `OneTimeSetUpAsync` because the hosted server never finishes starting. Reverted, with a comment at the site recording why. - **§17.10 and §15.10** (both low + PLAUSIBLE) — each needs a wider refactor than this PR: a consistent NodeId-constant rename across ~6 cross-reference sites, and replacing `Location` with a serializable equivalent for incremental cache equality. ## Spec cross-check The encoding/schema changes were checked against the specification: OPC 10000-6 §5.2.7 (32-bit EncodingMask, one bit per optional field in order, padded), the union `SwitchField` encoding, OPC 10000-3 §8.56 (AccessRestrictionType bit mask), §6.2.8 and §5.6.2/§5.6.5 (ValueRank), §8.51 (StructureField name == BrowseName). `GenerateStackTests.OptionalFieldBitsAgreeBetweenBsdAndGeneratedEncoder` pins the BSD presence-bit order against the generated encoder's `LogRecordFields` so the two cannot drift apart again. ## Related Issues _No tracking issue — this PR implements the SourceGeneration sections of a bug audit. Happy to open one if maintainers prefer._ ## 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. > Note on the test checkbox: verified on **net10.0 only** — `UA.slnx` builds clean (0 warnings, 0 errors) and `Opc.Ua.SourceGeneration.Core.Tests` (3915), `Opc.Ua.SourceGeneration.Tests` (184), `Opc.Ua.Di.Tests` (425, including the `RobotOpenUsdE2eTests` end-to-end fixture that catches address-space content changes) and the `EventRecordFieldReaders` tests (16) all pass. .NET Framework has **not** been run locally; CI needs to confirm it. 🤖 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-09-13 19:07:55 +02:00
Marc Schier	f5ad124891	Additional fuzzing plus fixes for every defect the campaigns found (#4459) ... # Description Brings the stack's fuzz testing to parity with the fork (`aio-opcua-sdk`), then fixes every defect the resulting campaigns exposed. The fork's fuzzing was used as the behavioural baseline: all **28 fork-local callbacks** and **13 OneFuzz registrations** are now accounted for by an explicit mapping, not just by a larger target count. The work extends the existing SharpFuzz/NUnit infrastructure rather than introducing another framework. ### Coverage - **84 continuous libFuzzer targets** and **81 AFL adapters** across Encoders, Certificates, Network and PubSub. - **PubSub area integrated** (Corpus/Tools/Tests siblings, solution entries) — it previously had host source only and was absent from the solution and replay infrastructure. - Segmented binary adapters, binary-to-JSON and XML-to-JSON conversions, standalone certificate loading, both chain-parser variants, CRL encoding and canonical TBS round-trips. - The 12 original fork message inputs are retained byte-identically alongside richer deterministic generators; the three historical crash/slow/timeout reproducers are unchanged (verified identical by SHA-256). ### Production fixes found by fuzzing These are real robustness and data-integrity bugs, each with a retained regression input: - `UadpBinaryReader.Advance` accepted attacker-controlled lengths on the receive path. Declared payload sizes are now authoritative bounded sections, with negative-count rejection and overflow-safe reassembly arithmetic. - PubSub JSON decoding leaked `InvalidOperationException` for invalid UTF-8. - `JsonEncoder.WriteExtensionObject` leaked `System.Text.Json.JsonReaderException`. - `JsonEncoder` silently emitted a lossy `{}` for an `ExtensionObject` whose TypeId could not be represented in JSON. - `JsonDecoder` wrapped a bodyless `ExtensionObject` envelope as a raw JSON body instead of preserving `ExtensionObjectEncoding.None`. - `Node.Encode` lost the null/empty collection distinction. - Binary/XML Variant matrix dimensions are prevalidated rather than relying on a caught `ArgumentException`. - `X509CRL.ReadTime` pre-validates strict RFC 5280 whole-second Zulu time forms. - `Utils.ParseCertificateChainBlob` disposes correctly on partial-chain failure. - **`nodeid.dict` was unloadable by libFuzzer** (empty token, unescaped quote). libFuzzer rejects the whole file on one bad line, so four parser targets silently executed **zero** inputs. Fixed and now guarded by dictionary validation. ### Infrastructure - `fuzzing/fuzz-targets.json` manifest recording adapters, corpora, dictionaries, oracles and fork-behaviour mappings, with `validate-fuzz-manifest.ps1` and **27 negative controls**. - Strict host/tools discovery and replay: `--list`, `--replay`, non-zero exits on unknown targets, missing or empty corpora, and failed callbacks (playback previously logged failures and returned success). - A real process watchdog for the timeout/slow regressions — NUnit `CancelAfter` cannot interrupt a synchronous fuzz callback. - Runner scripts made non-destructive (unique work directories, no generic `bin`/findings deletion) and corrected to the native driver's single-`--target_arg` apphost contract. - Opt-in `OneFuzz=true` class-library publication producing a relocatable, **uninstrumented** drop, validated by exact callback discovery, dependency closure, corpora and dictionaries. - CI: fuzz replay matrix plus a public drop gate, triggered by corpus-, dictionary- and script-only changes. ### Verification | Gate | Result | |---|---| | Bounded campaigns, all 84 targets (1,000 inputs, seed 1) | 84/84, 84,000 executions, zero findings | | OneFuzz drop | 84/84 published callbacks replayed, zero skips | | Fuzz replay net10.0 / net8.0 / net9.0 | 9,470 passed each, 0 failed | | Fuzz replay net48 | 8,996 passed, 0 failed (Network/PubSub are modern-only) | | `Opc.Ua.Types.Tests` / `Opc.Ua.Core.Tests` | 9,800 and 4,442 passed, 0 failed | | Retained regression assets | 132 `crash-*` inputs replayed by every target each run | ### Known gaps (deliberately not closed) - **Fidelity is not enforced against the pipeline crash corpus.** The differential binary-to-JSON targets assert both robustness (no unhandled exception, expected of arbitrary bytes) and fidelity (a decoded value re-encodes equivalently, a property of well formed values). The pipeline supplies 22,293 blobs collected for the targets that existed before this PR, none of which asserted fidelity; 522 of them violate it, and that count did not move across two rounds of genuine fixes because each fix only lets the mutator reach the next representability gap. Rather than weaken the assertion, the two contracts are separated: fidelity failures raise `EncodingFidelityException`; curated assets under `Assets/Repo`, the curated `Testcases` corpus and every continuous libFuzzer campaign stay strictly enforced, while externally supplied assets are gated on robustness only and their fidelity findings are reported with counts and bounded base64 reproducers. - **Internal MSFT OneFuzz canary has not been run.** It needs an owner-supplied ownership profile, pipeline authorisation and corpus snapshot; none were available. The drop builds and validates locally and records `serviceCanaryPerformed: false`. A build or upload is not a successful worker run. - **Legacy JSON wire forms are a partial mapping.** The fork accepts its four original `TypeId`/`Body` messages; this stack rejects that retired envelope (measured JSON-only control: fork `JsonEncoder` 630/1356 lines vs 0/1007 here). Recorded as an explicit unresolved mapping rather than an exemption. `LegacyNonReversible` is a RawData-derived profile and is not counted as independent coverage. ## Related Issues _No tracking issue was supplied for this work; happy to open one and link it if maintainers prefer._ - Fixes #github-issue-number, ... ## 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 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. > Note on the checklist: the fuzz replay matrix, `Opc.Ua.Types.Tests` and `Opc.Ua.Core.Tests` were run locally on **net48** and **net10.0** (plus net8.0/net9.0) and all passed. CI and CodeQL results are not yet available for this branch, so those boxes are left unchecked. The CLA box is for the author to confirm.	2026-09-10 05:56:31 +02:00
romanett	ececea15c8	Reimplement the GDS ApplicationsNodeManager as a generated fluent node manager (#4434) ... # Description Builds on #4428, which is now merged; this branch is rebased onto master. The GDS `ApplicationsNodeManager` becomes a source-generated `[NodeManager]`: the companion model, its loader and the fluent plumbing are emitted, and what stays hand-written is the behaviour. Getting there needed three things from the shared surface, which are the first two commits. ## Commits **1. Add the fluent hooks a hand-written manager needs at startup** - `OnReadRolePermissions` / `OnReadUserRolePermissions` on the node builder. Every other `NodeState` `On*` hook was already fluent; these were not, and a server that grants access on something the static model cannot express has to compute the permission set per request. - `Node<TState>(TState)` returns a builder over a node the caller already holds, for nodes materialised after `Configure` resolved its graph. - `OnAddressSpaceReadyAsync` on `FluentNodeManagerBase` — the one seam between the address space existing and the wiring being applied, for managers whose wiring depends on asynchronous setup. `Configure` is a `partial void` and cannot await. `DiNodeManager` had already invented the hook privately, so its declaration becomes an `override`. **2. Let a `[NodeManager]` bind to a model a referenced assembly supplies** A model belongs to the assembly that emits its types, which leaves a node manager for it homeless whenever that assembly cannot reference `Opc.Ua.Server` — a model-only package shared with clients, for instance. Binding was rejected twice over: the design was skipped before the binding was matched (`MODELGEN010`), and fluent-accessors-only mode refused to run alongside manager generation (`MODELGEN014`). The binding is now matched before the skip decision, and a matched binding puts that design into accessors-only mode. A design nothing binds to is still skipped, so this cannot start duplicating types by accident. The emitted manager also gains what a real manager needs: `DefaultNamespaceUris()` plus a protected constructor taking a replacement set; `GenerateDefaultConstructor=false` to suppress the two-argument form; its own logger category; and the call to `OnAddressSpaceReadyAsync`. Generated fluent accessors also stop re-enabling nullable warnings — a bare `#nullable enable` undid the shared header's `disable warnings`, so any model with a structure-typed method argument reported CS8600 on every generated `out T` in a project that builds warnings as errors. **3. Reimplement the GDS `ApplicationsNodeManager` as a generated node manager** The design stays owned by `Opc.Ua.Gds.Common`; the server assembly only binds a manager to it. **NodeIds do not move.** `NamespaceIndexes[0]` must stay the application record namespace — it is the one the application database, the certificate request store and the base allocator mint ids in — so the constructor passes the namespace order explicitly rather than adopting the generated model-first default. A test pins both ends against a live server. Four overrides go, leaving `OnAddressSpaceReadyAsync` and `Dispose(bool)` — no address-space overrides at all: | Removed | Why it was unnecessary | | --- | --- | | `New` | The base allocator already mints into `NamespaceIndexes[0]`, with a thread-safe counter. The override existed only because `Create(assignNodeIds: true)` asks the factory with the declaration id still set; `AssignInstanceNodeId` nulls it and re-asks, documented as being there for exactly this allocator. | | `AddBehaviourToPredefinedNodeAsync` | Its `AuthorizationServiceType` branch re-wired a node that was already wired, and its `KeyCredentialServiceType` branch had nothing to wire — the model ships that folder empty. | | `GetManagerHandleAsync`, `ValidateNodeAsync` | Verbatim reimplementations of the base that also blocked the fluent base's virtual-node support. | | `DeleteAddressSpaceAsync` | A `// TBD` passthrough. | Everything else — all sixteen directory methods with their self-administration permissions, the certificate groups' concrete types and trust-list flags, and the authorization service (created through this PR's `Add` surface and wired in the same place) — is one `Configure` pass. **4. Bind the certificate groups from the `Configure` pass** Acquiring a certificate group and binding it to its nodes are different kinds of work: the first is I/O (`InitAsync` opens certificate stores and creates CA certificates), the second only touches the address space. Each now runs in the phase it belongs to, which puts the whole of the wiring in one place. Ownership no longer rides on the binding. A group joins `m_ownedCertificateGroups` the moment it is constructed, before `InitAsync` can throw, so a group that fails half way through is still disposed — which the previous code could not manage, keying ownership off a dictionary it filled only after initialization succeeded. > Commit 3's message says this move is impossible and blames `PushTest`. **That was a wrong diagnosis, and this commit supersedes it.** Two real defects were hiding behind those failures: - **`Create(..., NodeId.Null, ..., assignNodeIds: false)` does not null the NodeId.** `NodeState.Create` overrides it only when handed a non-null one, so a custom group node kept the namespace-0 id of its type declaration. `AssignInstanceNodeId` hid this by re-asking the factory with the id nulled; the builder validates instead, and rejected it. The node is now created with an id in the manager's own namespace, as the `Default` authorization service already was. - **The custom certificate group node was not browseable.** It answered `GetCertificateGroups` and `ReadNode`, but a type-filtered recursive browse from the Objects folder never returned it — which OPC 10000-12 §7.8.2 requires of a certificate group. Staging the node through the builder gives it the reference it was missing. ### Two `PushTest` methods change — please review these closely The second defect had been masking what those tests actually did. Selecting the group positionally, `groups[3]` resolved to the server's **own `ServerConfiguration` group** while the custom one stayed invisible, so `UpdateTrustListOfCustomGroupAsync` and `AddRemoveCertOfCustomGroupAsync` were exercising a *transactional* trust list under a custom-group name. Browse results, before and after: | # | Before (baseline) | After | | --- | --- | --- | | 0-2 | GDS Directory groups (`ns=3`) | GDS Directory groups (`ns=3`) | | 3 | **`DefaultApplicationGroup` `i=14156`** — the server's own | **`MyCustomGroup` `ns=2`** | | 4 | — | `DefaultApplicationGroup` `i=14156` | They now resolve the group by browse name — which asserts it is browseable at all — and expect the immediate-apply semantics the GDS actually gives the groups of its own `Directory`: `CloseAndUpdate` writes the stores at once and reports `applyChangesRequired == false`, so the `ApplyChanges` calls go away (they would return `Bad_NothingToDo`). The read-back assertions are what prove the writes landed, and they are unchanged. This is a genuine loss of the *transactional* trust-list coverage those two methods were accidentally providing — the server's own group is still covered by the other `PushTest` methods, which target it deliberately. **5. Select `GetCertificates` results by type instead of by position** `GetCertificatesAsync` checked entry zero of a parallel-array result. Which entry that is depends on the order the server reports its certificate types in, an empty result throws `IndexOutOfRange` rather than failing an assertion, and a second type going missing or unparseable would not be noticed. Every returned pair is now checked, and a failure names the certificate type. **6. Drop the `.Common` suffix from the three GDS projects** Every other companion specification ships as `<Family>` / `<Family>.Client` / `<Family>.Server` — Di, ISA95, Positioning, Robotics, Vision, WotCon, XRegistry. GDS was the exception, with a `.Common` suffix that named nothing. | Before | After | | --- | --- | | `src/Opc.Ua.Gds.Common` | `src/Opc.Ua.Gds` | | `src/Opc.Ua.Gds.Client.Common` | `src/Opc.Ua.Gds.Client` | | `src/Opc.Ua.Gds.Server.Common` | `src/Opc.Ua.Gds.Server` | **No source changes were needed.** `RootNamespace` was already the final name in all three projects — the namespaces have always been `Opc.Ua.Gds`, `Opc.Ua.Gds.Client` and `Opc.Ua.Gds.Server`, and only the assembly and package ids disagreed. Nothing compiling against these assemblies changes a `using`. The package ids do change, which is consumer-visible, so `docs/migrate/2.0.x/packages.md` gains a renamed-packages section — it is the document that undertakes to cover NuGet renames — and the migration skill's package table gains the two rows a 1.5.378 consumer needs. Build infrastructure that names the projects or their output moves with them: both solution files, `expected-packages.txt` (re-sorted — it is maintained alphabetically), both signing manifests, the reference-server Dockerfile, the metapackage nuspecs, and the analyzer's package dependencies. `preview-pack.slnx` is derived from `UA.slnx` and needs no edit. The migration analyzer's shim tree mirrors the source layout by its own documented convention, so `Gds.Client.Common/` becomes `Gds.Client/`. Prose naming past work is left alone — plan 23 still records that its deferral happened during the "GDS Client.Common modernization", which is the name that effort had. ## Validation `net10.0`, on master + these four commits, from a clean GDS PKI: - `UA.slnx`: 0 errors, 0 warnings in changed files (the solution's 30 pre-existing warning lines are CA1861/CA1850/CA1307 in `XRegistry`, `Types.Tests` and `WotCon.Tests` — same rules and projects as the base). - `Opc.Ua.Gds.Tests`: 1,117 passed, 0 failed, 54 skipped — identical to the base branch. - `Opc.Ua.Server.Tests`: 4,961 passed, 0 failed, 5 skipped (measured before commit 4, which touches only the GDS assembly and its tests). - `Opc.Ua.SourceGeneration.Tests`: 169 passed. - `Opc.Ua.SourceGeneration.Core.Tests`, NodeManager/Generators/Fluent: 84 passed; `GenerateCode`: 21 passed, 4 skipped. - `Opc.Ua.MigrationAnalyzer.Core.Tests`: 10 passed, 3 skipped. - After the rename: assemblies build as `Opc.Ua.Gds.dll` / `.Client.dll` / `.Server.dll`, with no `*.Common.dll` produced anywhere. - Commits 1-3 build standalone. - `Opc.Ua.Gds.Server.Common` also builds for `net48` and `netstandard2.1` (checked before the rebase onto #4428). Not yet run: the full `Opc.Ua.SourceGeneration.Core.Tests` suite (~3,800), and the older target frameworks since the rebase. ## Checklist - [x] Existing tests pass - [x] New tests cover the new surface (role permissions, resolved-node builder, referenced-model binding, constructor/namespace control, GDS namespace invariant, custom-group node namespace) - [ ] Full Core.Tests sweep and net48/netstandard2.1 re-run after the rebase 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-09-09 10:41:06 +02:00
romanett	105508bcbd	Mint runtime NodeIds through a single DefaultNodeIdFactory (#4433) ... # Description Nodes created at runtime now get their NodeIds from a single `DefaultNodeIdFactory` rather than from a `New` override per NodeManager. The factory derives a deterministic identifier from a canonical, length-prefixed browse path, which is injective where the classic `{parent}_{child}` concatenation was not — `A_B` + `C` and `A` + `B_C` both produced `A_B_C` — and it also records the parent's identifier type and qualifies cross-namespace segments by URI, so identifiers no longer shift with namespace-table ordering. A NodeManager selects its style declaratively instead of writing assignment code: ```csharp NodeIdFactory = NodeIdFactory.WithMode(NodeIdAssignmentMode.Counter); ``` `NodeIdAssignmentMode` offers `String` (the canonical path verbatim, the default), `Numeric` / `Guid` / `Opaque` (that path through SHA-256), `Counter` (sequential, for nodes whose browse paths repeat — per-session diagnostics, inference jobs, rediscovered assets) and `None`. The counter starts above `0x40000000`, out of reach of any authored model, so a long-running server cannot walk a runtime instance onto a type node. `New` overrides are gone from every NodeManager the stack ships except two whose identifiers are a domain scheme rather than an assignment policy: `FileSystemNodeManager`, whose NodeIds encode the file path they resolve back to, and `RoboticsNodeManager`, whose build coordinator reserves identifiers across managers with ownership tracking. **Namespace rule.** Every identifier is minted into the NodeManager's own namespace. Neither the parent's namespace nor the browse name's is consulted: a parent can belong to a companion-specification model whose NodeIds are fixed by its NodeSet (`DeviceSet` in the DI namespace), and a browse name only names the type that declared the child. NodeManagers whose instance namespace is not their first one rebase with `WithDefaultNamespaceIndex`. **Types vs instances.** An authored NodeId on a node that stands on its own is kept. A node hanging off a parent is re-minted, because it reached the factory through `AssignNodeIds` walking a subtree copied from a type declaration. Keeping those would alias every instance onto the type's own nodes, and the predefined-node index takes the last writer, so the type would quietly become an instance rather than the clash being reported. New `ISystemContext.CreateInstance(...)` builds an instance of a generated type and rebases its subtree through `AssignInstanceNodeId` — the path the generated `CreateInstanceOf<Type>` helpers already took. The AI NodeManager was the one place calling `NodeState.Create` directly, so its roots kept the NodeId their state object is born with (the type's own); it now uses the shared path. ## Breaking changes - `IAsyncNodeManager` extends `INodeIdFactory` and gains `AddNode` and `AddRootNotifier`. This lets the fluent surface mint and register through the interface instead of type-testing for `AsyncCustomNodeManager` and silently falling back to a concatenated identifier when the test failed — that fallback was itself a source of divergent NodeIds. - `AsyncNodeManagerAdapter` delegates `New` to the wrapped NodeManager, so **`CustomNodeManager2` behaves exactly as before**: it mints nothing and a node keeps whatever NodeId it already has. Servers built on it are unaffected. ## Notes for reviewers - `docs/NodeIdAssignment.md` is new and describes the whole mechanism: the factory contract, identifier formats, type declarations vs instances, the generated helpers and their parameters, per-NodeManager behaviour, and an inventory of every member involved. - The three commits are independently revertible. - Two pieces that look redundant are deliberately kept, with the reasoning recorded in the doc: `assignInstanceNodeIds` (it *suppresses* minting, where the rebase rule *forces* it) and the generator's `NodeId.Equals(TypeNodeIdConstant)` guard (it distinguishes a caller-assigned NodeId from a declaration's). - Fluent builder tests previously passed only because of the removed fallback; they now run against a real `DefaultNodeIdFactory` through a shared `FluentTestNodeManager` helper. ## Related Issues _No tracking issue yet._ ## 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 were on **net10.0 only** — .NET Framework was not exercised > locally. Suites run green on an idle machine: Server 4950/4955, GDS > 1116/1170, WotCon 1225/1225, Vision 484/485, DI 424/424, AI 174/174, > ISA95 136/136, Positioning 69/69, RobotIntent 22/22. One unrelated > pre-existing failure remains in `Opc.Ua.Types.Tests` > (`EveryEmbeddedExampleMatchesItsRecordedHash`) — a WoT fixture checked > out with CRLF, fixed by > `git add --renormalize tests/Opc.Ua.Types.Tests/Wot/Assets`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 14:37:19 +00:00
romanett	668310f21f	Make the fluent node manager's configure/seal path genuinely async (#4442) ... # Description The fluent node-manager pipeline had two synchronous chokepoints, and two independent efforts have now hit both of them. `Configure` is a `partial void`, so it cannot `await`. A manager whose wiring depends on asynchronous setup had to invent a second hook outside the pipeline, create its own builder there, and hand-roll `CreateFluentBuilder(...).Configure(...).Seal()` inside it. `Seal()` was synchronous, so nothing a registration staged during `Configure` could be completed by awaiting. Registrations that needed to await either blocked — `EventSourceRegistry` did `.GetAwaiter().GetResult()` on the manager's root-notifier registration — or could not be expressed at all. ## What changed **`FluentNodeManagerBase.ConfigureAsync(INodeManagerBuilder, CancellationToken)`** is the awaitable wiring seam. It runs once per activation, with the manager's attached builder, immediately *before* the synchronous `Configure` partial(s) — so instances it materialises are in the address space by the time `Configure` wires callbacks against them. The default implementation is a no-op. It is a `virtual` method rather than a second `partial` declaration because a partial method can be optional (`partial void`, no return value) or awaitable (an extended partial method, which must be implemented) — not both, and the generator would otherwise have to detect which one the user declared. `partial void Configure(INodeManagerBuilder)` keeps working unchanged; a manager uses either hook or both. **`NodeManagerBuilder.Seal()` becomes `SealAsync(CancellationToken)`.** Sealing is now the single point where registrations that could not finish inside `Configure` get their turn to await. Every seal site awaits it, so a later change can move registry *setup* onto the same seam. **`DiNodeManager` now runs the standard pipeline itself** — create and attach the builder, `await ConfigureAsync`, `RegisterAuthoredNodesAsync`, `CompleteConfigureAsync`, the post-setup runner, then `await builder.SealAsync` — instead of handing subclasses a bare callback and leaving them to build and seal on their own. DI subclasses gain node registration and the reverse-reference pass, which they previously skipped. Sealing deliberately comes **after** the post-setup runner. The fluent registries are owned by the manager, not by a single builder, and sealing starts them — so sealing first locks post-setup configurators out of registering simulation loops of their own, which is exactly what `AddRobotics`' configurators do. Sealing before the runner makes `MinimalRobotServer` fail to start with *"Cannot add a simulation loop after the registry has started"*. Worth knowing for follow-up work: with several builders attached to one manager, the first seal freezes the shared registry for all of them. ## Workarounds removed - **`DiNodeManager.OnAddressSpaceReadyAsync`** — deleted. Its three overriders (`RoboticsNodeManager`, and the `PumpNodeManager` / `GeneratorNodeManager` samples) now override `ConfigureAsync` and no longer hand-roll `CreateFluentBuilder(...).Configure(Configure).Seal()`. - **`EventSourceRegistry`'s `.GetAwaiter().GetResult()`** on `AddRootNotifierAsync` — gone. `Publish(..., RegisterAsRootNotifier: true)` now stages the registration and `SealAsync` drains it, so the wait happens on an awaited path. A source whose root-notifier registration fails is rolled out of the registry before the failure surfaces, and a registry disposed before its builder sealed drops what it staged. - **`Isa95NodeManager.ConfigureStatusEvents()`** — the synchronous second builder pass is now `ConfigureStatusEventsAsync(CancellationToken)`, awaited from `CreateAddressSpaceAsync`. (`ConfigureCatalogChanges()` stays synchronous: it starts a background task and never seals a builder, so there was nothing to convert.) - **`IRoboticsBuildContext.Seal()`** — now `SealAsync(CancellationToken)`. These really were `NodeManagerBuilder.Seal()` calls behind a same-named method. The state transition still happens under the context lock; the builder's asynchronous completion runs outside it, so no `await` is taken while the lock is held. - The Pump / Generator incremental registration callbacks (`onRegistered`) changed from `Action<T>` to `Func<T, CancellationToken, ValueTask>` so their per-instance builder pass can await its own seal. No sync-over-async was introduced anywhere. `Dispose(bool)` is unchanged and stays non-blocking; teardown remains in `DeleteAddressSpaceAsync`. `NodeManagerBuilder.Seal()` and `IRoboticsBuildContext.Seal()` are removed rather than kept as synchronous overloads. A synchronous seal that silently skipped the asynchronous completion would reintroduce exactly the failure mode this change exists to remove. ## Generator `NodeManagerTemplates` emits, in order: predefined nodes load → `await ConfigureAsync(__m_builder, cancellationToken)` → `Configure(__m_builder)` and the typed `Configure` → `RegisterAuthoredNodesAsync` → `CompleteConfigureAsync` → `await SealConfigurationAsync(__m_builder, cancellationToken)`. `NodeManagerGeneratorTests` asserts that order literally, and that neither `__m_builder.Seal()` nor the synchronous `SealConfiguration(__m_builder)` is emitted. ### Merging with #4432 #4432 landed on `master` while this was in review and reworked the same seam from the other side: it split `Seal()` into `SealGraphAuthoring()` + `StartSimulations()` so a manager can replay `NotifyNodeAdded` *between* the two halves — sealing first stops a lifecycle handler authoring nodes nothing would register, starting the simulations last stops a simulated value change preceding its own node''s `OnNodeAdded` — and added `FluentNodeManagerBase.SealConfiguration(builder)` as the one call the generator emits. Both intents are kept. That split is precisely why the asynchronous completion could not simply live inside `SealAsync`: a manager reaching the seam through the split path would silently skip it, which is the failure mode this PR exists to remove. So the asynchronous half became its own step: - `NodeManagerBuilder.CompleteSealAsync(ct)` drains the staged root-notifier registrations, then starts the simulations. `SealAsync(ct)` is `SealGraphAuthoring()` followed by it. - #4432''s synchronous `StartSimulations()` is gone, folded into `CompleteSealAsync`. It had exactly one production caller after the merge, and leaving it standing would have kept a second, synchronous way to activate a builder that skips the staged registrations — the very hazard this PR removes. Activation is now one step with one entry point; the *split* itself stays, because the replay has to sit between the halves. - `SealConfiguration` becomes `SealConfigurationAsync(builder, ct)`: seal the graph, replay `NotifyNodeAdded`, then `CompleteSealAsync`. Activation still happens last, so #4432''s ordering guarantee holds unchanged. ## Consumers unblocked - **#4434 ("Reimplement the GDS ApplicationsNodeManager as a generated fluent node manager")** adds `OnAddressSpaceReadyAsync` to `FluentNodeManagerBase` purely because, in its own words, *"`Configure` is a `partial void` and cannot await"*. That hook is unnecessary once `ConfigureAsync` exists — the GDS manager overrides `ConfigureAsync` instead, and the ordering it wants (predefined nodes → async setup → `Configure`) is what the generator now emits. #4434 has not merged yet, so it can drop its addition rather than have it simplified afterwards. - **Branch `romanett/fluent-node-behaviors`** moved `SimulationRegistry`, `EventSourceRegistry`, `MonitoredSourceRegistry` and the alarm wiring onto `AttachToType`/`Attach` for *release* only, because activation ran from `CompleteConfigureAsync` and most managers only called the synchronous `Seal()`. Every seal site now reaches `SealAsync`, so registry and alarm *setup* can move onto behavior activation too — the remaining half of that work. ## Tests - New `DiNodeManagerConfigureAsyncTests` drives the real `DiNodeManager.CreateAddressSpaceAsync` and asserts the hook runs once with an attached builder, that nodes it stages are registered, that their references to externally owned nodes are published, and that the builder is sealed before `CreateAddressSpaceAsync` returns. - `PublishTests` now pins the root-notifier deferral contract: registration is staged during `Configure`, lands only on the seal, and draining twice does not double-register. - Every `Seal()` call site in the test suites became `await ...SealAsync()`, including the three #4432 added in `NodeSetImportBuilderTests` and `SimulationBuilderExtensionsTests`. ## Related Issues No tracking issue was opened for this; it is the shared prerequisite extracted from #4434 and branch `romanett/fluent-node-behaviors`, both of which had independently worked around the same synchrony. Happy to open one if maintainers want the ADR trail. ## 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. ### What was actually run locally `UA.slnx` builds clean. Post-merge on **net10.0**: `Opc.Ua.Server.Tests` 5020/0 and `Opc.Ua.SourceGeneration.Core.Tests` 3813/0. Pre-merge the same run plus `Opc.Ua.Di.Tests` 425/0, `Opc.Ua.Robotics.Tests` 627/0, `Opc.Ua.ISA95.Tests` 136/0, `Opc.Ua.Positioning.Tests` and `Opc.Ua.Robotics.Intent.Tests` were all green; `MinimalRobotServer` was also started directly to confirm the hosted robotics path comes up. Three caveats, so the unchecked box above is not a mystery: the full suite was **not** run against .NET Framework locally; `Opc.Ua.OpenUsd.Tests` has one pre-existing failure on this machine (`StructuredCartesianCoordinatesAreAccepted` expects `"1.5"` and gets `"1,5"` — a German-locale decimal separator, unrelated to this change); and one pre-existing `CA1861` warning comes from `NodeSetImportIntegrationTests.cs`, which is byte-identical to `master` on this branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 12:01:56 +00:00
Marc Schier	da7412c429	Add native xRegistry events and distinct Resource versions (#4396) ... ## Summary - support the xRegistry 0.6.0 distinct Resource/Version hierarchy: `Registry -> Group -> Resource -> Versions -> Version` - instantiate stable logical Resource nodes separately from exact Version nodes while preserving existing Version NodeIds - delegate logical Resource FileType operations and Version-view metadata to the selected default Version with handle pinning and shared writer reservations - make Resource and Version delete behavior role-stable and use `MetaEpoch` / `Epoch` respectively - use stable Resource SourceNodes, exact Version SourceNodes and the `Server -> Registry -> Group -> Resource -> Version` notifier chain - keep a model-metadata-gated client fallback for 0.5 flat Servers; 0.6 Servers expose no legacy aliases - target `HasWoTProjection` at the real stable logical Resource node - use `AsyncCustomNodeManager` for the registration, fast-path, and federation managers, with awaited lifecycle, node mutations, event delivery, and cleanup - handle startup rollback, reentrant deletion callbacks, undisclosed file handles, and retryable cleanup without blocking unrelated file operations ## Authoritative dependencies - companion specification: OPCF-Members/spec-drafts#32 at `d3399fca025025a73e5a6b599f850a81d985c060` - OPC UA binding: xregistry/spec#568 - xRegistry event semantics: xregistry/spec#563, merged as `66ee31e47fbadff6ab982ae1516f7d9065e46b64` - companion NodeSet SHA-256: `369f5f3f1814b333e4eeb52bfb4b6a730cdff372505a5f91a54c94f0016ac4cf` ## Review feedback - keep the WoT Connectivity information-model designation at `1.1`; draft revisions update the publication date without changing the model version - eliminate all 198 unique CA1861 diagnostics across 54 files and remove seven obsolete project suppressions - fix the analyzer-clean touched-path CA1307 and the hierarchy follow-up CA1846/CA1851 diagnostics without dropping legacy-TFM compatibility - recover the reconcile queue after callback faults/cancellation and continue accepted pending changes - remove migration guidance for the xRegistry managers, which are new in 2.0 ## Compatibility - public WoT Connectivity model designation remains `1.1` - generated event-record ABI, constructors, options, NodeIds, and stored documents remain compatible - the generic xRegistry managers are new in 2.0 and implement `IAsyncNodeManager`; use `IAsyncNodeManagerFactory` and the async lifecycle hooks (see `docs\XRegistry.md`) - persistence schema and document blobs remain compatible unless noted below ## Validation - merge validation at `980422f76` with upstream `e8bec27d5`: 454 targeted WoT/materialization/model/registry cases and all 327 XRegistry cases passed on both net10.0 and net48; all 29 net10 aggregation sample cases passed - feedback regressions at `308e8ba74`: 55 targeted queue and live registry event/projection/lifecycle cases passed on each of net10.0 and net48 - prior CI: all 266 checks passed at `6489a0908`; the unchanged Ubuntu Core certificate-counter test and dependent jobs passed on the one-time retry ([run 34117304035, attempt 2](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/34117304035/attempts/2)) - `Opc.Ua.XRegistry.Tests` — 327 passed on net10.0 and net48 for migration commit `32c914aaf` - `Opc.Ua.WotCon.Tests` net10.0 — 1348 passed - `Opc.Ua.WotCon.Bindings.Tests` net10.0 — 683 passed - `Opc.Ua.SourceGeneration.Core.Tests` net10.0 — 3807 passed, 8 skipped - `Opc.Ua.SourceGeneration.Tests` net10.0 — 145 passed - `Opc.Ua.SourceGeneration.Stack.Tests` net10.0 — 94 passed - affected XRegistry and WoT client/server projects build successfully - xRegistry NodeSet is byte-identical to the companion artifact - repository-wide CA1861 inventory — 198 unique diagnostics across 54 files reduced to zero - targeted changed-fixture validation — 1,098 passed, 3 skipped, 0 failed - constrained full `UA.slnx` net10.0 Release build — CA1861 = 0 - `Opc.Ua.XRegistry.Client` net472 and netstandard2.1 builds — 0 warnings, 0 errors - independent shared-array safety review — no significant issues - `git diff --check` The final review regressions cover concurrent reconcile/open, synthetic logical handle collisions across default switches, cross-session Close isolation, shared writer reservations, logical FileType property/change propagation, logical Labels delegation, role-fixed deletion, event sources, model-version gating, and stable `HasWoTProjection` targets. The companion specification and protocol binding are reviewed in the linked PRs above. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5c79a1ed-6dab-4522-a9fc-6548caa78ad5 Copilot-Session: b605797f-a97a-4cd0-8f12-016d33c2fd88 Copilot-Session: 7ef369a5-2307-4f71-8a11-15af0e72f240 Copilot-Session: e716d455-8397-4d22-9c5a-398847ab7317	2026-09-08 04:46:40 +00:00
romanett	30ed2390a8	Import NodeSet2 documents into a fluent node manager (#4432) ... ## What `INodeManagerBuilder.Import(UANodeSet, INodeSetImportFactoryProvider?)` lets a source-generated (or hand-written fluent) node manager overlay a NodeSet2 document that is only known at runtime onto the model it was generated from. Every document imported during one `Configure` pass forms a single batch that is linked **exactly once** after the pass. A node may therefore declare a `ParentNodeId` that lives in another document of the batch, or a node the manager already owns — which is what makes an overlay possible. Imported nodes resolve through `Node(...)`/`NodeFromTypeId(...)` immediately, so they can be wired in the same pass; they are registered with the manager when `CompleteConfigureAsync` runs. ### Public surface - **+1 method**: `INodeManagerBuilder.Import(UANodeSet nodeSet, INodeSetImportFactoryProvider? factoryProvider = null)`. - **+3 contracts** in `Opc.Ua.Server.Nodes`: `INodeSetImportFactoryProvider`, `INodeSetImportFactory`, `NodeSetImportDiscriminator` (TypeDefinition / MethodDeclaration / NodeId). - In `Opc.Ua.Export`: a public `UANodeSet.Import` overload taking a `NodeSetImportStateFactory`, and a public batch `LinkParentChildRelationships(context, nodes, NodeSetImportLinkOptions)`. These replace what would otherwise have been an `InternalsVisibleTo("Opc.Ua.Server")` on `Opc.Ua.Types`. - `NodeState.IsExplicitlyDefinedChild(context, child)` and `protected DetachExplicitlyDefinedChild(child)`, used to tell a generated slot apart from an ordinary child. ### Typed states, no reflection The generator emits `{Model}NodeSetImportFactoryProvider` — one factory per model type/declaration, each calling a concrete constructor for an *empty* state — and makes the generated node manager implement `INodeSetImportFactoryProvider` by delegating to it. So the builder resolves the provider from the manager (`NodeManager as INodeSetImportFactoryProvider`); `Import` also accepts one directly, and the generated manager exposes `partial void AddNodeSetImportFactories(List<INodeSetImportFactory>)` for dependency models. An imported node of a generated type materializes as its generated `*State` subclass with no type lookup, which keeps the path NativeAOT-safe. Because the factories create empty states, an imported instance carries exactly the children its document declares. ### Reconciliation When an imported child lands in a slot the generated parent declares (same BrowseName), it replaces the generated placeholder: the displaced node and every descendant the document does not carry over are removed from the manager, and references to them are retargeted at the replacement. Replacing a node the same `Configure` pass already wired is rejected with `BadInvalidState` rather than silently dropping the wiring, and sealing a builder with an unregistered import batch is rejected too. ### Also - `RuntimeNodeSetNodeManager` now uses the same importer: its documents are one batch, so a node may declare a parent in another document of the group regardless of parse order, and duplicate NodeIds across documents are detected while parsing. - Linking resolves an imported child's reference type from its own inverse reference (previously always `HasComponent`/`HasProperty`). - Method import keeps the document's `UserExecutable`; it previously copied `Executable` into it. View import keeps `EventNotifier`. ## Tests - `NodeSetImporterTests` (15) — discriminator selection, generic fallbacks, field/value/reference fidelity against a generic import, cross-document linking, typed replacement of inherited and argument slots, deferred linking preserving an application `Handle`, duplicate NodeIds, factory misuse. - `NodeSetImportBuilderTests` (15) — staging and lookup before registration, batch linking, attaching to an existing node, provider resolution, placeholder replacement with reference retargeting, configured-placeholder rejection, seal/completion guards. - `NodeSetImportIntegrationTests` (5) — a generated `[NodeManager]` overlaying its own model on a running server: typed states through the generated provider, cross-document linking, placeholder replacement, and browse through a session. - `NodeManagerGeneratorTests` (+4) — the emitted provider and the manager's delegation, including that no factory uses `Activator.CreateInstance`/reflection. - `NodeSetAotTests` (+1) — the public state-factory import under NativeAOT. Full `Opc.Ua.Server.Tests` (4946), `Opc.Ua.SourceGeneration.Core.Tests` (3821), `Opc.Ua.SourceGeneration.Tests` (165) and `Opc.Ua.Types.Tests` pass locally on net10.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 23:12:43 +00:00
romanett	1aed93bd03	Make the predefined node create lifecycle cancellable (#4437) ... # Description Registration of predefined nodes could not be cancelled: `CreateAsPredefinedNode` ran the whole create lifecycle without a token, and a cancellation that landed inside `AddPredefinedNodeAsync` could still leave the manager half-indexed. **Cancellable create lifecycle.** Adds a `NodeState.CreateAsPredefinedNode(ISystemContext, CancellationToken)` overload. The token is threaded through the internal `CallOnBeforeCreate` / `CreateInternal` / `CallOnAfterCreate` walk, is checked between nodes and on each convergence pass, and is passed to every `OnAfterCreate` override. Cancellation leaves the nodes already completed in the created state, so a later call resumes only the remaining ones. The existing single-argument overload is unchanged and delegates with `default`. **Shared registration helper.** `CustomNodeManager` and `AsyncCustomNodeManager` each carried an identical private `CompleteCreateLifecycleForRegistration`. Both are replaced by one internal `NodeStateLifecycle.CompleteForRegistration`. `AddPredefinedNodeAsync` now checks cancellation on entry, propagates it into lifecycle completion, and checks again before `IndexPredefinedNode`, so a cancelled registration cannot leave a node indexed, half-created, or a behaviour replacement partially wired. _(An earlier revision also added `RemoveExplicitlyDefinedChild` overrides on `MethodState` and `BaseDataVariableState`. Those were dropped in 45c6b3eb0: #4432 covers the same case inside `CreateOrReplaceInputArguments` / `CreateOrReplaceOutputArguments` / `CreateOrReplaceEnumStrings` by assigning the typed replacement directly, so this PR no longer touches those files.)_ The three `<see cref="NodeState.CreateAsPredefinedNode"/>` references (two in `src`, three occurrences in a generator template) are qualified with the parameter list because the new overload makes the unqualified form ambiguous (CS0419). Public API added: one overload. No new virtual members. ## Related Issues No tracking issue — this is a self-contained extraction from the in-progress node-source lifecycle work, kept separate so the cancellation semantics can be reviewed on their own. ## Testing - `dotnet build UA.slnx -c Debug` — 0 errors, no new warnings. - `dotnet format whitespace UA.slnx --verify-no-changes` on the changed files — clean. - net10.0: `Opc.Ua.Types.Tests` 9704 passed, `Opc.Ua.Server.Tests` 4965 passed, `Opc.Ua.SourceGeneration.Core.Tests` 3809 passed — 0 failures (skips pre-existing). New coverage: six `AsyncCustomNodeManagerTests` for the registration cancellation seams (already-cancelled token, an already-created node not being indexed, the helper's own check, cancellation raised from the behaviour callback for both the replacement and same-instance paths, and that removing a node neither deletes it nor repeats its create lifecycle), three `NodeStateTests` for token propagation, pre-cancellation and resumption between children. ## 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. — ran the three affected suites on net10.0 only; .NET Framework and the remaining suites are left to 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 Opus 5 <noreply@anthropic.com>	2026-09-07 20:04:42 +00:00
romanett	39602bccb1	Add node creation to the fluent node manager builder (#4428) ... # Description `INodeManagerBuilder` gains a node **creation** surface alongside its existing lookup surface, so a `Configure` partial can fill a namespace without a NodeSet or a ModelDesign. Twelve new members, implemented as ordinary public members on `NodeManagerBuilder` (new partial file `NodeManagerBuilder.Authoring.cs`): | Member | Creates | | --- | --- | | `AddFolder(name, parentId)` | a `FolderState` (`Organizes`) | | `AddObject(name, parentId, typeDefinitionId)` | a `BaseObjectState` | | `AddVariable<TValue>(name, parentId)` | a `BaseDataVariableState` whose `DataType`/`ValueRank` come from `TValue` | | `AddMethod(name, parentId)` | an executable `MethodState` | | `Add<TState>(node, parentId)` | an already-constructed state of any `NodeState` subclass | | `Add<TState>(factory, parentId)` | a state built by a factory that receives the resolved parent | | `AddRoot<TState>(node)` | a root, with its existing references left alone | | `TryGetNode(nodeId, out node)` | lookup across created-but-not-yet-registered nodes and predefined nodes | Each `Add*` for a browse name takes a `string` (qualified with the manager's default namespace) or a `QualifiedName` carrying an explicit nonzero namespace index — eight overloads plus the four above. ```csharp partial void Configure(INodeManagerBuilder builder) { INodeBuilder<FolderState> machines = builder.AddFolder("Machines"); builder.AddVariable<double>("Pressure", machines.Node.NodeId) .OnRead(() => m_sensor.Pressure); builder.AddMethod("Reset", machines.Node.NodeId) .OnCall(ResetAsync); } ``` Three properties make this usable straight from `Configure`: - **NodeIds are final before the builder comes back.** Every `Add*` runs the node — and its whole subtree — through the manager's `INodeIdFactory` before returning, so `OnRead`/`OnWrite` registrations, which key off `NodeState.NodeId`, stay valid once the node is registered. - **Creation is staged, not immediate.** Created nodes are held until the manager calls `RegisterAuthoredNodesAsync`. That is what lets a node name a sibling created moments earlier, and it keeps the builder usable before the manager has finished building its address space. - **Custom state types stay typed.** `Add<TState>` returns `INodeBuilder<TState>`, so a hand-written `NodeState` subclass keeps its type through the fluent chain. Staging also makes the builder's own lookups see created nodes: browse paths, `NodeId`, `TypeDefinitionId` and `DataType` resolution all consult the staged graph before falling back to the manager's predefined nodes. ### Registration ordering A new `protected FluentNodeManagerBase.RegisterAuthoredNodesAsync(builder, ct)` hands the staged roots to `AddPredefinedNodeAsync`. It runs **after** the `Configure` delegates and **before** `CompleteConfigureAsync`, so the reverse-reference pass sees the new nodes and mirrors their references to externally owned nodes (typically the ns=0 `Objects` folder) into `externalReferences`. Three call sites emit it in that position: - the source-generated `CreateAddressSpaceAsync` (`NodeManagerTemplates`) - the hosting `FluentNodeManager` (`FluentNodeManagerFactory`) - `RuntimeNodeSetNodeManager`, which additionally re-runs `AddReverseReferencesAsync` because its first pass happens before `Configure` A builder that created nothing registers nothing, so the call is safe to make unconditionally. The surface is available to **every** fluent host — there is no opt-in gate and no second builder interface. The generated typed builder (`FluentBuilderGenerator`) forwards the new members like the rest of `INodeManagerBuilder`. ### Supporting change `AsyncCustomNodeManager.PrepareAuthoredNodeIdsForRegistration` (internal) assigns an id to every node in a subtree that still lacks one and pulls namespace-0 children into the root's namespace. It differs from the existing `PrepareInstanceNodeIdsForRegistration`, which only rebases a subtree that collides with a declaration. It is idempotent. ## Related Issues - Follow-up tracked in #4429 (adopting this surface in the remaining hand-rolled node managers, which each need a base-class change first). - No tracking issue for the API itself yet; happy to open one if maintainers would like the design recorded as an ADR before review. ## Notes for reviewers Three judgement calls worth a look: 1. **Always-on rather than gated.** The feature is available to every fluent host with no enable call. There is no `INodeSource`-style concept here to withhold it from, so an always-on capability is the same behaviour with one fewer state flag. Misuse is still rejected: `Add*` after `Seal()` or after registration throws `BadInvalidState`, a NodeId in a namespace the manager does not own throws `BadNodeIdInvalid`, and a parent in one of the manager's *own* namespaces that was never created throws `BadNodeIdUnknown`. 2. **Detaching from the external-parent proxy goes through `AddChild`/`RemoveChild`.** The factory overload hands the factory an identity-only proxy when the parent belongs to another node manager, then has to drop the parent link the factory established. `BaseInstanceState.Parent` has an `internal` setter scoped to `Opc.Ua.Types`, so this pair is the only public route. Commented as such at the call site. An alternative would be widening that setter or adding `InternalsVisibleTo` — happy to switch if preferred. 3. **`PrepareNodeIds` does not re-walk per descendant.** The root pass already covers the subtree, so per-child calls would be no-ops on a quadratic walk; descendants are only namespace-validated. One inherited behaviour the new tests pinned down: `BaseObjectState(parent)` sets `ReferenceTypeId = HasComponent` in its own constructor, so a node built via the factory overload lands under the `Objects` folder with `HasComponent`, while the plain `AddObject` path picks `Organizes`. That asymmetry predates this PR; flagging it in case it is worth normalizing separately. `Import(UANodeSet)` is deliberately **not** part of this change — it belongs to NodeSet import rather than node creation. ## Test results 19 new tests in `tests/Opc.Ua.Server.Tests/Fluent/NodeManagerBuilderAuthoringTests.cs` cover NodeId assignment before return, parent-by-NodeId nesting, the default Objects-folder placement and its inverse reference, custom state types, both factory paths (authored parent and external-parent proxy), `AddRoot` reference preservation, `TryGetNode`, id-keyed read handlers firing on the registered instance, browse-path/NodeId visibility of created nodes, and every rejection path listed above. Run against **net10.0** (`-p:CustomTestTarget=net10.0`): | Suite | Result | | --- | --- | | `Opc.Ua.Server.Tests` | 4926 passed, 0 failed, 5 skipped | | `Opc.Ua.SourceGeneration.Core.Tests` | 3809 passed, 0 failed, 8 skipped | | `Opc.Ua.SourceGeneration.Tests` | 165 passed, 0 failed | `UA.slnx` builds with 0 errors and no new warnings. ## 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. (New `Creating nodes from scratch — the Add* surface` section in `docs/NodeManagers.md`, plus TOC entry.) - [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.0 only so far** (results above); .NET Framework has not been run locally yet. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. — pending first CI run. - [ ] 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-07 07:53:38 +02:00
Marc Schier	02ea934a31	Complete fluent node manager extensibility (#4418) ... # Description Complete the fluent node-manager extensibility surface needed by dynamic and externally backed address spaces. This change: - adds virtual node families with cheap NodeId ownership checks, async materialization, per-operation caching, custom browsing, and family-level callbacks; - adds monitored-item pre-creation decisions, complete lifecycle hooks, async batch hooks, stack-managed custom monitored items, and subscription-gated polling; - exposes the startup `ApplicationConfiguration` to generated manager partials through `FluentNodeManagerBase`; - fixes typed builder proxy support, concrete object wrapper types, and fully-qualified generated model composition; - reports unresolved `[NodeManager]` namespace expressions as `MODELGEN035` instead of silently dropping them; - documents the new APIs and compatibility behavior. ## Related Issues - Fixes #4397 - Fixes #4398 - Fixes #4399 - Fixes #4403 - Fixes #4404 - Fixes #4405 ## Validation - `Opc.Ua.Server.Tests` on `net10.0`: 4,886 passed, 5 skipped. - `Opc.Ua.SourceGeneration.Core.Tests` on `net10.0`: 3,807 passed, 8 skipped. - `Opc.Ua.SourceGeneration.Tests` on `net10.0`: 164 passed. - Runtime and generator regression selections passed on `net48`. - Review-feedback validation: 376 fluent tests passed on `net10.0`; 22 focused tests passed on `net48`. - The CI timing regression was repeated five times on `net48` and once on `net10.0`. - All 266 applicable GitHub Actions and Azure Pipelines checks, including CodeQL and coverage, pass on `83c571423`. - AppVeyor reports an immediate zero-duration no-run failure because `appveyor.yml` whitelists only the `appveyor` branch; it did not execute this PR. - `UA.slnx` builds successfully for `net10.0`; affected project builds are warning-free. - The repository-wide `dotnet test UA.slnx` invocation is currently blocked by the solution's mixed VSTest/Microsoft.Testing.Platform configuration under the .NET 10 SDK, so the affected test projects were run individually. ## 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. - [x] I have addressed **all** PR feedback received. --------- Copilot-Session: afaa67c9-fc97-4185-a785-edd54a937d95	2026-09-05 09:25:49 +02:00
Marc Schier	e7542b7f5f	Fix generated structured defaults and dependency metadata (#4414) ... # Description Fix two source-generator paths that lost variable values or metadata: - XML `ExtensionObject` defaults without a `TypeId` now resolve their body through the encodeable factory's registered XML qualified name, decode to the generated structure, and retain the XML encoding identifier. - The existing preview `ModelDependencyV1` payload now carries effective child access and user-access values, minimum sampling interval, historizing state, and serialized default-value XML across referenced assemblies. The dependency validator rehydrates this metadata before inherited instance factories are generated. Explicit non-null extension-object TypeIds remain authoritative, unknown XML bodies remain raw XML, and explicit design files remain authoritative over referenced payloads. ## Related Issues - Fixes #4401 - Fixes #4402 ## Validation - Focused decoder, payload, validator, and Roslyn producer/consumer regressions on `net10.0` and `net48` - `tools/SourceGeneration.slnx` Debug builds on `net10.0` and `net48` - `tools/SourceGeneration.slnx` Release build on `net10.0` - Scoped `UA.slnx` Debug build on `net10.0` ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Copilot-Session: 66e6d342-d4a0-4d53-822b-524982c62f15	2026-09-04 12:42:32 +00:00
Marc Schier	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
Marc Schier	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
Marc Schier	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
romanett	14fba2d120	Fix instance TypeDefinition across dependency ModelDesign boundaries (#4355) ... # Description The model source generator crashed with `MODELGEN003` / `NullReferenceException` whenever a `ModelDesign` instantiated an `ObjectType` (or `VariableType`) declared in a **dependency** design file, as soon as that type had children. The sibling of #4332 on the instance-declaration path: #4335 linked dependency *data types*, but an *instance* whose `TypeDefinition` lives in another design still crashed in `NodeStateGenerator` on the type's inherited children. Root cause: dependency design files load with `validateDictionary: false`, so `ValidateInstance` / `ValidateType` never ran on them and their types' children kept `TypeDefinitionNode` / `DataTypeNode` at `null`. When the target instance merged those children into its hierarchy (`BuildInstanceHierarchy` re-walks the dependency type's `Children.Items`), the null references were copied along, and `GetNodeStateClassName` dereferenced them while building the node state factory list. Changes: - A new best-effort `LinkDependencyInstances` pass in `ModelDesignValidator` links dependency-design instances the way `ValidateInstance` / `ValidateType` do for the target model: the `TypeDefinitionNode` / `DataTypeNode` of the types' children and standalone instances (recursively), variable `DataType` / `ValueRank` inheritance from the variable type, `DefaultValue` decoding, `VariableTypeDesign.DataTypeNode` restrictions, and method arguments — including creation of the `InputArguments` / `OutputArguments` argument properties so method children materialise into the target hierarchy exactly like local ones. Resolution is best effort (unresolvable references stay `null`) and NodeSet2 dependencies remain excluded, consistent with `LinkDependencyDataTypes`. - The pass runs alongside `LinkDependencyDataTypes`: for upstream dependencies after all of them are loaded and before the target validates (so the hierarchy merge picks up the links), and for downstream dependencies after they load. - `docs/ModelDependencies.md` documents the instance linking. Tests (all verified to fail with the exact reported NRE stack before the fix): - `CrossModelInstanceTypeDefinitionTests` (Core, 4 tests): the issue's minimal repro end to end; an equivalence test asserting the cross-model instance emits the **identical node-state factory set** as a locally declared type (this is the issue's stated expectation, and it drove the method-argument-property part of the fix); the flipped upstream/downstream ordering; and validator-level assertions that the merged hierarchy carries linked `TypeDefinitionNode` / `DataTypeNode` references, including the data type inherited from a dependency `VariableType`. - `CrossModelInstanceTypeDefinitionGeneratorTests` (Roslyn driver, 2 tests): the issue's csproj repro shape — both `AdditionalFiles` orderings, asserting no diagnostics and that the combined generated output of both models **compiles** (the instance factories reference the dependency model's typed state classes). Verified locally on net10.0: `Opc.Ua.SourceGeneration.Core.Tests` 3800 passed, `Opc.Ua.SourceGeneration.Tests` 141 passed, `Opc.Ua.SourceGeneration.Stack.Tests` 94 passed; 0-warning Release build of `Opc.Ua.SourceGeneration.Core` (all TFMs incl. netstandard2.0); `dotnet format whitespace --verify-no-changes` clean. This removes the last generator blocker for source generating the instance model of the `Workshop/DataTypes` sample (OPCFoundation/UA-.NETStandard-Samples#814): its `ParkingLot` / `DriverOfTheMonth` are instances of types declared in the other design file. ## Related Issues - Fixes #4353 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-31 09:47:08 +00:00
romanett	24ba4855e6	Fix [DataType] generator Clone() dropping every inherited field of a derived structure (#4354) ... # Description The `[DataType]` source generator emitted a `Clone()` that created a fresh instance and copied only the fields declared on the annotated type itself, so every field inherited from an `IEncodeable` base was silently dropped. Since the server clones node values while loading an address space, a derived structure served by a node manager came back out of a Read with its inherited fields empty, even though encode/decode was correct. This PR makes the `[DataType]` path emit the same `MemberwiseClone`-based shape as the model-based generator: ```csharp /// <inheritdoc/> public override object Clone() { return (DerivedStruct)this.MemberwiseClone(); } /// <inheritdoc/> public new object MemberwiseClone() { DerivedStruct clone = (DerivedStruct)base.MemberwiseClone(); // deep copy of the own reference-typed fields return clone; } ``` `Clone()` delegates to a generated `public new object MemberwiseClone()` that starts from `base.MemberwiseClone()` — a shallow copy of the whole object, inherited fields included — and then deep copies the type's own reference-typed fields (encodeables, `DataValue`, `Variant`, `ExtensionObject`). Derived classes hide `MemberwiseClone` and chain into the base implementation, so deep-copy semantics compose across the hierarchy, including across assemblies into model-generated base types (the `BicycleType` scenario that surfaced this). The two generator paths now agree. Because the clone now starts from a full shallow copy (`base.MemberwiseClone()` for classes, `with { }` for records), the per-field clone emitter only writes the deep-copy assignments; the redundant per-field plain copies are gone. This also fixes a latent side bug in the class path: init-only simple fields were silently dropped by the fresh-instance clone as well. Records were already correct (`with { }` carries every field) and are unchanged in shape. Tests: - `GenerateClassCloneUsesMemberwiseClone` / `GenerateDerivedClassCloneCarriesInheritedState` assert the emitted shape for root and derived classes. - `DerivedClassCloneKeepsInheritedFields` runs the exact repro from the issue through the Roslyn driver, emits the assembly, and executes `Clone()` at runtime to prove the inherited field survives (verified on net10.0 and net48). Verified locally: `Opc.Ua.SourceGeneration.Core.Tests` (3798 passed), `Opc.Ua.SourceGeneration.Tests` (140 passed), `Opc.Ua.SourceGeneration.Stack.Tests` (94 passed), and `Opc.Ua.Client.Tests` (2161 passed, exercises the regenerated in-tree `[DataType]` record types). The in-tree `[DataType]` consumers (Opc.Ua.Client, both GDS common projects) regenerate and build without warnings. ## Related Issues - Fixes #4352 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-30 22:10:27 +00:00
romanett	766c5558e8	Fix structure subtyping across dependency ModelDesign boundaries (#4335) ... # Description The model source generator crashed whenever a `ModelDesign` declared a structure that subtypes a structure from **another** design file (`MODELGEN003` with a `NullReferenceException`), and the cross-assembly `[assembly: ModelDependency]` flow failed even earlier with `The BaseType reference ... is not the expected type: DataTypeDesign`. Structure inheritance across model boundaries — the primary use case of companion-spec layering — could not be source-generated at all. Three root causes, all in the dependency-loading path of the validator: 1. **Dependency design files were never linked.** They load with `validateDictionary: false`, so their `DataTypeDesign`s kept `BasicDataType` at the enum default (`Boolean`) and their fields' `DataTypeNode` stayed `null`. `BinarySchemaGenerator`'s inherited-field merge then dereferenced the null `DataTypeNode`, and `XmlSchemaGenerator` / `DataTypeGenerator` silently emitted `xs:boolean` / `bool` from the default classification. 2. **Dependencies that import the target crashed at load.** Each generation group receives every design file of the compilation as a dependency, including downstream models that import the group's target — those loaded *before* the target they import and failed resolving their `BaseType`. This produced the confusing `AdditionalFiles`-order-dependent failure modes. 3. **The payload flow never reached ModelDesign targets.** `ModelCompilation` supplied `referencedDependencies` only to the NodeSet2 pass, never to the design-file pass, and payload-materialised data types lacked the classification and field links needed to serve as base types of local structures. Changes: - `ModelDesignValidator.ValidateModel` loads dependencies in two phases around the target: upstream models first (topological, referenced models before their referrers), then the target, then models that import the target. - A new `LinkDependencyDataTypes` pass classifies dependency data types (`IsStructure` / `IsEnumeration` / `BasicDataType`) and resolves field `DataTypeNode` / `Parent` best effort. It runs for payload-imported declarations and upstream design-file dependencies before target validation, and for downstream dependencies after they load. NodeSet2 dependencies are excluded — `NodeSetToModelDesign` already links and classifies them with its own semantics. - Payloads for models that are backed by a design file in the compilation are skipped — the explicit file is authoritative (previously, importing both crashed with duplicate symbolic ids). - Payload-materialised data types honor the payload's `IsEnumeration` flag. - The design-file `GenerateCode` pass now receives the referenced assemblies' dependency payloads (`ModelCompilation` + new optional `referencedDependencies` parameter). - `docs/ModelDependencies.md` updated accordingly. Tests (all verified to fail with the exact reported errors before the fix): - `CrossModelStructureSubtypeTests` (Core, 6 tests): the issue's minimal repro end to end — asserting the emitted `.bsd` carries inherited fields with correct types and `SourceType`, the `.xsd` extends the dependency structure (not `xs:boolean`), and the generated C# derives correctly — plus the reversed target/dependency order, both payload flows at generation and validator level, and a three-level chain (target → payload structure → design-file structure). - `CrossModelStructureSubtypeGeneratorTests` (Roslyn driver, 3 tests): both `AdditionalFiles` orderings compiling the generated output, and the full cross-assembly flow (producer assembly generated from the upstream design; consumer resolves the base structure from the reference's payload without re-emitting it). Verified locally on net10.0: `Opc.Ua.SourceGeneration.Core.Tests` 3786 passed, `Opc.Ua.SourceGeneration.Tests` 135 passed, `Opc.Ua.SourceGeneration.Stack.Tests` 94 passed; 0-warning Release builds of `Opc.Ua.SourceGeneration.Core` (all TFMs incl. netstandard2.0), both Roslyn generator variants, Stack and Tester. This unblocks the `[NodeManager]` migration of the DataTypes workshop sample (OPCFoundation/UA-.NETStandard-Samples#757). ## Related Issues - Fixes #4332 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-08-30 17:22:34 +00:00
romanett	e61ad4b306	Fully support sub-state machines in the fluent state-machine builder (#4322) ... # Description Part 16 §B.3 places `HasSubStateMachine` on the parent **state** node, but the fluent builder attached it to the state-machine **root** because `FluentFiniteStateMachineState` never materialized state nodes at all. Investigating #3821 showed the gap was wider than the reference placement: 1. **No state nodes existed.** The NodeId written into `CurrentState/Id` — `new NodeId(stateId, ElementNamespaceIndex)` — was **dangling**: no node with that id was in the address space. 2. **`AvailableStates` was never populated**, so `GetAvailableStatesAsync` returned empty against a fluent-built server. 3. **The whole client discovery chain was therefore dead.** `ObserveEffectiveStateAsync` enumerates `AvailableStates` and browses `HasSubStateMachine` from each state node — it found nothing. The client side was already spec-correct; only the server deviated. 4. **`HasSubStateMachine` is non-hierarchical** (a subtype of `NonHierarchicalReferences`, `i=32`). Using it as the sub-SM's `BaseInstanceState.ReferenceTypeId` left the sub-SM unreachable from a hierarchical browse of the machine. ## What changed **Element nodes are materialized.** Definition-mode machines now create one `StateType` node per declared state and one `TransitionType` node per declared transition — each a `HasComponent` child carrying `StateNumber` / `TransitionNumber`, with the §B.4 `FromState` / `ToState` / `HasEffect` references and `HasCause` added by `WithCause`. `AvailableStates`, `AvailableTransitions` and `LastTransition` are populated, so `CurrentState/Id` and `LastTransition/Id` resolve to real, browsable nodes. NodeIds are per-instance, so several machines built from one definition coexist. **`HasSubStateMachine` moved to the state node**, with the inverse `SubStateMachineOf`, and the sub-SM is attached as a `HasComponent` child (fixing 4). **Core gains element-id mapping hooks.** `GetCurrentStateId` / `UpdateStateVariable` / `UpdateTransitionVariable` hard-coded the numeric form and were non-virtual, so a subclass could not redirect them. They now route through `public virtual` `GetStateNodeId` / `GetStateId` / `GetTransitionNodeId` / `GetTransitionId`, whose defaults reproduce the previous `(elementId, ElementNamespaceIndex)` convention **exactly** — every stack-shipped and generator-emitted machine is unaffected, and there is a regression test asserting that. **Generated machines report their model namespace.** The generator now emits an `ElementNamespaceUri` override on `FiniteStateMachineType` subtypes, taken from the nearest type that actually *declares* state children — a subtype that only adds behaviour inherits its elements, and their namespace, from its base. **Ordering and validation fixes.** `WithSubStateMachine` and `WithInitialState` are now order-independent (previously, attaching a sub-SM before setting the initial state left the child suspended until the parent left and re-entered the state — silently). `WithSubStateMachine` validates `parentStateId`, and the builder rejects duplicate element browse names, since element NodeIds are now derived from them. ## A defect this surfaced Code that assembles nodes with the generated `CreateInstanceOf<Type>` factories never calls `NodeState.CreateAsPredefinedNode`, so `OnBeforeCreate` / `OnAfterCreate` are skipped for the entire subtree — and `OnAfterCreate` is the only place a state machine resolves its element namespace. It is also where `ConditionState` binds its `Enable` / `Disable` / `AddComment` handlers and `AlarmConditionState` binds the shelving methods, so the impact is broader than state machines. This PR fixes only the two call sites it depends on — DI's `SoftwareUpdateFacetWiring` and Robotics' `RoboticsOperationBuilder`. **The repository-wide audit is deliberately left out of scope and tracked in #4320** (24 files call these factories; none completed the lifecycle). The Robotics regression had no test coverage at all, so an assertion on `CurrentState/Id` was added and verified to fail without the fix. ## Breaking changes - Sub-state machines are **no longer browsable from the state-machine root**. Callers passing a machine's `ObjectId` to `GetSubStateMachineAsync` must pass a state NodeId instead — from `GetAvailableStatesAsync` or a snapshot's `CurrentStateId`. - `CurrentState/Id` for fluent machines changes from the numeric form to the materialized node's id. Code reading a numeric id out of it should call `FiniteStateMachineState.GetStateId` rather than parsing the identifier. - Generated machines qualify `CurrentState/Id` / `LastTransition/Id` with their companion-spec namespace instead of the OPC UA one. Documented in `docs/WhatsNewIn2.0.md` and `docs/StateMachines.md`. ## Notes for reviewers - Adding `virtual` to the `FiniteStateMachineState` members is a binary (not source) breaking change — assumed acceptable on the 2.0 line, but worth confirming against the compatibility policy. - Address-space cost is one node plus one property per declared state and transition, per machine instance; called out in the docs for servers instantiating many machines. - `AvailableTransitions` is modelled as a `HasComponent` `BaseDataVariable` (matching the standard NodeSet and what the client's browse path expects), not a Property. ## Related Issues - Fixes #3821 - Surfaced #4320 (node create lifecycle audit — not addressed here) ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ### Test runs (net10.0) | Suite | Result | |---|---| | `Opc.Ua.Server.Tests` (full) | 4734 passed, 5 skipped | | `Opc.Ua.SourceGeneration.Core.Tests` | 3780 passed | | `Opc.Ua.SourceGeneration.Tests` | 132 passed | | `Opc.Ua.SourceGeneration.Stack.Tests` | 94 passed | | `Opc.Ua.Types.Tests` | 8763 passed | | `Opc.Ua.Robotics.Tests` | 540 passed | | `Opc.Ua.Robotics.Intent.Tests` | 22 passed | | `Opc.Ua.Di.Tests` | 424 passed | | `Opc.Ua.ISA95.Tests` | 136 passed | | `Opc.Ua.Aas.Tests` | 670 passed | | `Opc.Ua.Positioning.Tests` | 69 passed | | `Opc.Ua.Client.Tests` (StateMachines) | 36 passed | Only net10.0 was run locally; the .NET Framework leg is left to CI, hence the unchecked box above. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-29 13:33:16 +00:00
romanett	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
Marc Schier	397db603db	Add OPC UA for Asset Administration Shell: the published OPC 30270 and v3 draft (#4244) ... # Description Implements the **Asset Administration Shell** companion specifications on top of `marcschier/wot-type-binding`, in two halves that a Server chooses between: * **AAS V3** — the [draft](https://github.com/marcschier/opcua-drafts/tree/main/companion-specs/AAS) (`http://opcfoundation.org/UA/I4AAS/v3/`), all sixteen conformance units: the metamodel, the registry built on the abstract xRegistry base, generational materialization, federation, packages with digest-bound integrity, the Digital Product Passport vocabulary, and the Annex F bridge to WoT Thing Descriptions. * **AAS V2** — the published [OPC 30270](https://reference.opcfoundation.org/specs/OPC-30270/full) (`http://opcfoundation.org/UA/I4AAS/`), which maps the AAS V2.0.1 metamodel: object model, ingestion of XML, JSON and AASX, materialization, server and client. The two are different metamodel generations rather than two revisions of one model — V2 keeps `Asset` as a first-class Identifiable, `identification.idType`, Views and ordered collections, none of which survive into V3 — so they are **alternatives**. A Server hosts one or the other; this is documented rather than enforced at startup. Each is named for the generation it maps (`Opc.Ua.Aas.V2`, `Opc.Ua.Aas.V3`, `AddAasV2Server`, `AddAasV3Server`), and what genuinely serves both carries no marker and sits at the root of `Opc.Ua.Aas`: the deterministic NodeId encoding and idShortPath conventions, `AasOptional<T>`, the digest rule, the materialization diagnostics, the AASX container walk, and the value and operation providers, which are keyed on `NodeId` and `Variant` and so carry no version at all. ## What is in here **New libraries**: `Opc.Ua.Aas`, `Opc.Ua.Aas.Server`, `Opc.Ua.Aas.Client`, `Opc.Ua.Aas.WoT`, plus `samples/Aas` and `docs/Aas.md`. **Enabling changes to existing code**, each needed by the above: * The OPC UA `Decimal` DataType added to `Opc.Ua.Types`, with its Part 6 §5.1.10 wire encoding. `BinaryDecoder` gained one internal member, `TryReadRemainingBodyBytes`, because a trailing field with no self-describing length cannot otherwise be read; `Decimal.Decode` is its only caller. Part 6 Table 3 types the unscaled `Value` as an **OctetString whose byte count is `Length` − 2**, not a length-prefixed `ByteString`, so it carries no length of its own — `DecimalEncodingTests` pins the body byte-for-byte against that mistake, and against the `Length ≤ 2` values the same table calls invalid. * The xRegistry projection engine extracted into `Opc.Ua.XRegistry.Server` and `Opc.Ua.WotCon.Server` migrated onto it (−848/+387 lines), with the WoT Connectivity tests passing **unedited**. * Two source-generator fixes, both of which made a model impossible to build (see below). ## Defects found and fixed along the way Running the stack end to end, rather than trusting per-area test suites, surfaced defects that no unit test caught. The ones worth a maintainer's attention: * **The generator emitted non-terminating constructors.** A structure field with no declared default was initialized with `new T()`, so a self-referential DataType (`AASReferenceDataType.referredSemanticId`) recursed until the stack overflowed and `AddOpcUaAas` could not load the model at all. The test host *died* rather than failing. Only the field that closes a cycle is now left null. * **The generator omitted `new` on a redeclared placeholder.** `AASOrderedSubmodelElementCollectionType` redeclares the `SubmodelElement` slot of its base type, and the generated `Add` method hid the base one without saying so, so the V2 model would not compile. No existing model could have hit either defect — both are build failures, so anything that builds today is unaffected. * **The metamodel half had never worked outside its unit tests**: the emitted NodeSet declared no reference-type aliases and was rejected by the loader; the Mandatory `Invoke` Method was never emitted, because a NodeSet is imported as written rather than instantiated from its type; the runtime bound at the authored namespace index instead of the one the Server assigns; and `MethodDeclarationId` was addressed in the wrong namespace. Every materializer test inspected the in-memory object graph, which cannot see whether the graph is loadable — two tests now import it exactly as `RuntimeNodeSetNodeManager` does. * **The client could not see any Property member.** It browsed `HasComponent` with subtypes, but `HasProperty` is a sibling under `Aggregates`, so reading a value failed and invoking failed argument validation. * **Every repeated registry write forked a duplicate group and resource** instead of adding a version, because the identifier was re-derived against a sibling set that already contained the entity itself. This also made the persistence bounds unreachable. * `.gitignore`'s `**/packages/*` silently swallowed the whole package-integrity implementation and its tests, which would have vanished on a fresh clone. ## Specification findings Nine findings against the V3 draft are fixed in [marcschier/opcua-drafts#75](https://github.com/marcschier/opcua-drafts/pull/75). The substantive one: clause 6.1.5's absent-versus-empty rule was **unsatisfiable** for five container fields that materialize as parent components with no node of their own, which made `AAS-LosslessRoundTrip` unachievable. OPC 30270 contradicts itself twice. Both are recorded in `AasV2ConformanceUnits.cs` and the documentation rather than smoothed over: Table 83 spells the unit `I4AAS MultiLangaugeProperty` where Table 85 spells it correctly, and Table 85 lists an `I4AAS Security` unit that Table 83 never defines. Table 84, which would assign profile URIs, is empty, so `ServerProfileArray` gains no I4AAS entry. ## Known limitation Calling an `Operation` **with arguments** answers `BadTooManyArguments`. Method resolution works from both the instance NodeId and the `MethodDeclarationId`, and the argument definitions are published, but `NodeState.AddChild` does not route through `MethodState.FindChild`, so a Method imported from a NodeSet leaves its typed `InputArguments` null. Closing this means changing how the shared NodeSet import claims typed children, which affects every consumer of runtime NodeSets and is out of scope here. It is documented in `docs/Aas.md` §11, the conformance matrix marks `AAS-OperationInvoke` partial rather than claiming it, and `samples/Aas` exercises the path and reports the status rather than working around it. V2 is unaffected — its `Operation` is an OptionalPlaceholder with no declared arguments. ## Related Issues - No tracking issue yet. Opening as a **draft** for design discussion first, per the ADR guidance below; happy to open one and link it once the shape is agreed. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ### Verification | Check | Result | |---|---| | AAS tests | **670 passed, 0 failed** on net472, net48, net8.0, net9.0, net10.0 | | Coverage | Client 80.8 %, Server 81.2 %, `Opc.Ua.Aas` 82.0 %, WoT 82.4 % — all above the 80 % gate | | Libraries | `Opc.Ua.Aas`, `.Server`, `.Client`, `.WoT` build clean on every target framework, zero warnings | | Regression | `Opc.Ua.Types` 8677, `Opc.Ua.WotCon` 1094, `Opc.Ua.XRegistry` 198, `Opc.Ua.Server` 4050 — all green | | Sample | `samples/Aas` builds warning-free and runs end to end | | Merge validation | Solution builds clean on net10.0 and net48; WoT Types 504, WotCon 1111, AAS 670, XRegistry 198 tests passed | | CI | All 99 checks green on `ff9298e44`; only the CLA signature status is outstanding | ### Review pass A full review against both specifications, and for API consistency and correctness, produced twelve fixes. Each one has a test that fails without it. | Defect | Where | Why it mattered | |---|---|---| | V2 projection aborted on **every** `File` read from a document | `AasV2EnvironmentRuntime` | The runtime bound `File/Value` unconditionally, the materializer emitted it only with content, and the builder throws `BadNotFound` for an unknown NodeId — aborting the whole registration. No reader ever populates `AasFile.File`, so no AAS V2 document could be hosted at all. | | `aas-suppl` anchored on the wrong part | `AasxPackageReaderCore`, `AasxPackageWriter` | IDTA and the Package Explorer anchor it on the environment part; we used the origin part. Every real `.aasx` returned zero supplementary files, silently. Affects both halves. | | Federation failed closed on pooled connections | `AasFederation` | `ConnectCallback` fires only for new connections, so the second fetch to an origin — and all of net472/net48/netstandard2.1 — failed with a security-flavoured error. The peer is now validated *inside* the callback, which makes reuse safe by construction. | | Malformed `Decimal` escaped as `ArgumentOutOfRangeException` | `BinaryDecoder` | A declared body shorter than `Scale` produced a negative read length. Two of the three test cases had been passing for the wrong reason. | | V2 and V3 cross-wired their value providers | `OpcUaAasServerBuilderExtensions` | Both registered `IAasValueProvider`; whichever was added first served both AddressSpaces. V2 now has its own contract, so co-hosting is harmless rather than silently wrong. `EnvironmentFolder` was also bound and ignored on the V2 path. | | Export filter skipped three nesting kinds | `AasEnvironmentExporter` | Content under `Entity`, `AnnotatedRelationshipElement` and `Operation` was exported unevaluated *and* stamped with a Digest presenting it as complete (clause 6.5.10). List authorization paths also shifted after a denial. | | Concealment held at the Method but not in the projection | `AasRegistryModel` | `GetSubmodel` answered `BadNotFound` while an anonymous Browse revealed the node, its identifier, semanticId and digest (clause 6.5.7). | | File handles were unsynchronized and unowned | `AasElementFileManager` | The async node manager holds no lock, so concurrent Calls raced a bare `Dictionary`; sequential handles let any Session read, seek or commit another's. Writes were also unbounded. | | Projection host was unimplementable outside the stack | `AasEnvironmentProjection` | The handle demanded a type with an internal constructor — our own tests used `FormatterServices.GetUninitializedObject` to get round it. Now mirrors WotCon's interface-typed registration. | | Embedded `FileType` omitted 7 of 10 Mandatory members | `AasEnvironmentMaterializer` (V2) | OPC 10000-3 6.4.4 requires them all; without `Size` and `OpenCount` a Client cannot size a read or detect an abandoned handle. | | Round-trip oracle ignored most of the model | `AasRoundTripComparer` | `Blob`, `File`, `ReferenceElement` and `BasicEventElement` had no case arm, so their fields compared equal by falling off the switch; `valueType`, `semanticId`, `entityType` and `Key.type` were never read. The clause 6.4 losslessness claim rested on it. | Known, unchanged: calling an `Operation` with arguments answers `BadTooManyArguments`, because `NodeState.AddChild` does not route through `MethodState.FindChild`. Documented in `docs/Aas.md` §11. Not addressed in this pass, and worth a maintainer decision: OPC 30270 §9.2 says all NodeIds of Nodes not defined in the companion specification **shall not** use the standard namespaces, and Table 87 marks a vendor-specific instance namespace Mandatory. Both halves currently mint instances in the companion namespace. Separating them touches NodeId minting, the runtime and the client together, so it is better done deliberately than folded into a review pass. This is a large change and touches shared code (`Opc.Ua.Types`, the source generator, the xRegistry projection). It is deliberately a draft: the enabling changes are separable if maintainers would prefer them split into their own PRs. The latest `master` is merged into this branch in `0dd0ff0e1`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8 Copilot-Session: c3d88950-4a65-4ff6-87d2-1f545a875c9b Copilot-Session: b8ab3763-b9bd-46ae-b0ef-045db26e5782	2026-08-18 08:11:47 -07:00
romanett	333de8305b	Add kitchen-sink coverage NodeSet2 with three load pipelines (#4251) (#4272) ... # Description Regression fence for the NodeSet2 import + code-generation surface. This adds a single, purpose-authored "kitchen-sink" NodeSet2 (`Opc.Ua.CoverageTest.NodeSet2.xml`) plus a small dependent secondary model that deliberately flip a broad set of the knobs defined by `UANodeSet.xsd` (every node class, every `Definition` shape, custom reference types, all five modelling rules, scalar/array/matrix values, a deep branching instance tree, and a wide set of reference relations). The same bytes are then loaded into **real** servers (`ServerFixture<T>` + `ReferenceServer`) through three independent pipelines and validated node-by-node, attribute-by-attribute, reference-by-reference: 1. **Source-generation, hand-written manager** - the XML is consumed at compile time via `<AdditionalFiles ModelSourceGeneratorPrefix="Opc.Ua.CoverageTest">`; the generated `AddOpcUaCoverageTest(context)` composes the predefined nodes inside a `CustomNodeManager2`, so every node materialises as its generated `NodeState` subclass. 2. **Runtime import** - the identical XML is imported at startup through `RuntimeNodeSetNodeManagerFactory` / `AddRuntimeNodeSet`. 3. **Source-generation, generated fluent manager** - a `[NodeManager]`-attributed partial makes the generator emit the whole `FluentNodeManagerBase` manager, factory and typed fluent-builder surface. A shared, data-driven `[TestCaseSource]` battery runs the identical assertions against all three servers, and a cross-pipeline equivalence fixture asserts the three address spaces agree. A dependent **second namespace** (`.../Secondary/`) exercises cross-namespace `HasTypeDefinition`, `HasSubtype` and `HasComponent` references in every pipeline. Authoring the model surfaced two latent source-generator bugs, both fixed here: - `NodeSetToModelDesign` crashed on a top-level `<ServerUris>` table (`StringTable.Append(null)` for the reserved local-server slot). Added a null guard plus a focused regression test (`NodesetServerUrisTests`). - `FluentBuilderGenerator` emitted colliding typed-builder accessors for any model with more than one structure encoding, because `DataTypeEncoding` objects hang off their `DataType` and surfaced as parent-less "root" instances sharing the reserved `DefaultBinary`/`DefaultXml`/`DefaultJson` symbolic names. They are encoding metadata, never fluent-wired, so they are now excluded from the typed builder. A few edges are intentionally not asserted for cross-pipeline equivalence because the pipelines legitimately differ (documented in `CoverageNodeSet/README.md`): type-dictionary / `DataTypeDescription` nodes (the generator regenerates schema rather than emitting them), a handful of ambiguous duplicate-hierarchical / View-`Organizes` edges, and runtime structure stand-ins (structure definitions are validated via the `DataTypeDefinition` round-trip instead). ## Related Issues - Fixes #4251 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-08-17 14:59:58 -07:00
romanett	d547747e22	Generate ReferenceNodeManager address space from a NodeSet2 model (#4220) ... # Description The CTT/compliance `ReferenceNodeManager` built its ~6,000 node address space entirely in imperative C#. This PR migrates it to a source-generated `[NodeManager]` partial driven by an exported NodeSet2 model (`ReferenceServer.NodeSet2.xml`), mirroring the `MinimalBoilerServer` sample and completing the goal of issue #4204. The address space is now materialized from the model by the source generator. The large orchestration body was kept but converted from create-based to **find-instead-of-create**: each `Create*` primitive now looks up the model-materialized node via `FindPredefinedNode<T>` and re-applies runtime values, DataAccess metadata, and write handlers, rather than constructing nodes from scratch. This keeps the deterministic value seeding and behavior wiring intact while the structure comes from the model. ## Notable details for reviewers - **Idempotent reference wiring.** The mesh references in the `References` folder are already present in the model, so the historic manual `AddReference` calls would throw on duplicates. Added a small `AddReferenceIfMissing` helper so those calls are safe. - **Non-string NodeIds.** The `NodeIds` folder assigns numeric / GUID / opaque NodeIds. The model exports those nodes with their final ids, so find-by-string-path no longer works; added `CreateVariableById(NodeId)` to locate them by their real id. - **Generator bug fix (worth a careful look).** `ModelDesignExtensions.GetValueRankAsCode` reconstructed `ValueRank` as `dimensions.Length + 1`, so a node with `ArrayDimensions="3,3"` was emitted as `ValueRank = 3`. This affected **any** NodeSet2 with a fixed multi-dimensional array (rank-3 matrices, `BadTypeMismatch` on whole-matrix writes, wrong decoded dimensions). Corrected to `dimensions.Length` and updated the unit tests that had codified the off-by-one. ## Validation - `ReferenceServerTest`: 44/44 on net10.0 and net48 - `Opc.Ua.SourceGeneration.Core.Tests`: pass on net8.0/net9.0/net10.0/net48 - Integration on net10.0: ComplexTypes, Client, History, Sessions, Subscriptions all pass - Clean build, no new analyzer warnings ## Related Issues - Fixes #4204 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>	2026-08-15 05:22:31 -07:00
romanett	6e0ed89d3f	Make UANodeSetSerializers.g.cs generation deterministic (#4252) ... ## Problem Regenerating `src/Opc.Ua.Types/Schema/UANodeSetSerializers.g.cs` via `UANodeSetSerializers.ps1` produced ~144 lines of no-op churn on every run. The sgen tool (Microsoft.XmlSerializer.Generator) emits the Reader `idNN_Name` field declarations and the matching `InitIDs()` initializers in a hash-seed-dependent order: the assigned id numbers are stable, but their emission order shuffles between runs. This made an otherwise no-op regeneration produce a large, noisy diff that was painful to review. ## Fix - Added a canonicalization pass in `UANodeSetSerializers.ps1` that sorts each of the two affected contiguous blocks (the `string idNN_Name;` field declarations and the `idNN_Name = Reader.NameTable.Add(...)` initializers) by numeric id ascending, so re-running on unchanged input yields byte-identical output. - The name regex was widened to `@"[^"]*"` so entries with an empty name (e.g. `id3_Item`) or URI-style names do not split the block into independently-sorted runs (which would keep it unstable). - Regenerated the committed `.g.cs` once through the improved script; the resulting diff is sort-only. ## Also fixed: tool download Replaced `Expand-Archive` (which rejects the `.nupkg` extension on Windows PowerShell) with `System.IO.Compression.ZipFile.ExtractToDirectory`, so a clean run with an empty tool cache works end-to-end on both Windows PowerShell 5.1 and PowerShell 7+. ## Validation - From an empty tool cache, the full download + extract + generate path succeeds. - A second run is a byte-identical no-op (`git diff --exit-code` = 0). - `dotnet build src\Opc.Ua.Types\Opc.Ua.Types.csproj -c Release -f net10.0` builds with 0 warnings and 0 errors. - All AOT post-processing is preserved: custom header, `#if NET5_0_OR_GREATER` wrapper, module IL2026/IL3050 suppressions, `[RequiresUnreferencedCode]`/`[RequiresDynamicCode]` on the InitCallbacks() overrides (Writer = RequiresUnreferencedCode only, Reader = both), pragmas, UTF-8 BOM, and CRLF line endings. No hand-edits to `.g.cs`; the fix lives entirely in the script so future regenerations stay stable. ## Also: Load UserAccessLevel from NodeSet2 model Bundled here because it regenerates `UANodeSetSerializers.g.cs`, which this PR owns. - `UANodeSet.cs`: adds the `{Member}Specified` XmlSerializer convention to `UAVariable.UserAccessLevel` (new `UserAccessLevelSpecified` companion; removed the `[DefaultValue((uint)1)]`) so an explicitly-present `UserAccessLevel` attribute is distinguishable from the schema default of 1. - `UANodeSetSerializers.g.cs`: regenerated - the writer now emits `UserAccessLevel` when `UserAccessLevelSpecified`, and the reader sets `UserAccessLevelSpecified = true`. Regenerating produced only these two behavioral changes plus removal of pre-existing empty `if (o.@MemberSpecified) { }` no-op blocks (see below); no `idNN` re-sort, so determinism held. - `UANodeSetHelpers.cs`: the runtime importer honors an explicitly-specified `UserAccessLevel` instead of always deriving it from `AccessLevel`. - Source generator (`ModelDesign.cs` `RawUserAccessLevel`, `ModelDesignExtensions.GetUserAccessLevelAsCode`, `NodeSetToModelDesign.cs`): emit the verbatim `UserAccessLevel` when the NodeSet sets it, otherwise derive from `AccessLevel` (matching the runtime importer). ### Generator normalization for `{Member}Specified` sgen emits an extra empty `if (o.@MemberSpecified) { }` no-op block near `WriteEndElement` for every member that uses the `{Member}Specified` convention. Adding the companion to `UserAccessLevel` would have introduced one such block, and the committed file already contained several for other members (`LastModified`, `AccessRestrictions`, `PublicationDate`). `UANodeSetSerializers.ps1` now strips these empty guard blocks during post-processing, so the generated file stays free of dead code and a re-run of the generator remains a byte-identical no-op. The real, non-empty guard around the `WriteAttribute` call is preserved. The ReferenceServer sample application of this feature is intentionally excluded and lands with #4220. ### Validation (UserAccessLevel) - `dotnet build src\Opc.Ua.Types\Opc.Ua.Types.csproj -c Release -f net10.0` and `dotnet build tools\Opc.Ua.SourceGeneration.Core\Opc.Ua.SourceGeneration.Core.csproj -c Release`: both 0 warnings / 0 errors. - `dotnet test tests\Opc.Ua.SourceGeneration.Core.Tests -c Release -f net8.0 --filter "FullyQualifiedName~NodeSetToModelDesign|FullyQualifiedName~UserAccessLevel"`: 16 passed, 0 failed. - Re-running the generator remains a byte-identical no-op. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-08-14 08:00:13 -07:00
romanett	34fe1a1567	Emit WriteMask from the model in the source generator (#4253) ... ## Summary Isolates a self-contained OPC UA source-generator bug fix, extracted from PR #4220 (which targets a different base branch) so it can merge independently against `master`. ## Fix: Emit node WriteMask from the model **Bug:** The `NodeState` source generator hardcoded `global::Opc.Ua.AttributeWriteMask.None` for both the `WriteMask` and `UserWriteMask` attribute replacements. As a result, attributes declared writable in a NodeSet2 model (for example `WriteMask=96`) were emitted as `None`, so writes to those attributes failed with `BadNotWritable`. **Change:** - `tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs` - the common node replacements now emit `root.GetWriteMaskAsCode()` for the `WriteMask` and `UserWriteMask` tokens instead of the hardcoded `AttributeWriteMask.None`. - `tools/Opc.Ua.SourceGeneration.Core/Schema/ModelDesignExtensions.cs` - adds a `GetWriteMaskAsCode(this NodeDesign node)` helper that returns `global::Opc.Ua.AttributeWriteMask.None` when `WriteAccess == 0`, otherwise `(global::Opc.Ua.AttributeWriteMask){writeMask}`. ## Tests - `tests/Opc.Ua.SourceGeneration.Core.Tests/Schema/ModelDesignExtensionsTests.cs` - adds `[TestCase]`-driven coverage for `GetWriteMaskAsCode` across representative write-mask values plus a null-node test. ## Validation - `dotnet build tools\Opc.Ua.SourceGeneration.Core\Opc.Ua.SourceGeneration.Core.csproj -c Release` - 0 warnings, 0 errors. - `dotnet test tests\Opc.Ua.SourceGeneration.Core.Tests\Opc.Ua.SourceGeneration.Core.Tests.csproj -f net8.0 -c Release` - 3775 passed, 0 failed, 8 skipped. ## Note on the Views fix (excluded) A second generator fix (preserve forward same-namespace `Organizes` references to `ViewDesign` targets in `NodeSetToModelDesign`) was intended to ship here as well, but it was excluded. That fix compensates for behavior introduced by #4220 where imported Views are promoted to top-level `ViewDesign` nodes and their `ParentNodeId` is cleared. On `master` that promotion does not happen (Views remain children of their folder), so the fix and its test do not apply and cannot be cleanly isolated. It will land with #4220. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>	2026-08-14 07:57:44 -07:00
romanett	cc4791e2fd	Import NodeSet2 Views as standalone top-level nodes in the source generator (#4259) ... ## Problem The NodeState source generator mishandled `UAView` nodes imported from a NodeSet2 model. When an exporter set a View's `ParentNodeId` to an organizing folder, the importer/generator absorbed the View into that folder's `Children` and tried to emit an invalid `AddChild(...)` call. Because `ViewState` derives from `NodeState` (not `BaseInstanceState`), a View can never be an `AddChild` component, so the generated code was invalid. ## Fix Make Views standalone top-level predefined nodes linked purely via `Organizes` references — mirroring how `DataType`/`ReferenceType` type nodes and `DataTypeEncoding` are handled: - **`tools/Opc.Ua.SourceGeneration.Core/Schema/NodeSetToModelDesign.cs`** — clear the `UAView` `ParentNodeId` so the View stays a standalone top-level node, keep the organizing folder's forward `Organizes` references to those Views, and carry the View's `ContainsNoLoops` attribute through to the `ViewDesign`. - **`tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs`** — return `null` for `ViewDesign` in the `AddChild` and `ReplaceChild` template paths so Views are never emitted as components. - **`tests/Opc.Ua.SourceGeneration.Core.Tests/Schema/NodeSetToModelDesignTests.cs`** — new `ViewImport` NodeSet constant and `ImportPreservesFolderOrganizesReferencesToViews` test verifying Views are imported as top-level nodes while the organizing folder retains its forward `Organizes` references to them. ## Validation - `dotnet build tools\Opc.Ua.SourceGeneration.Core` (Release): 0 warnings / 0 errors. - `dotnet test tests\Opc.Ua.SourceGeneration.Core.Tests` (net8.0, filtered to `NodeSetToModelDesign`): all 17 tests pass, including the new `ImportPreservesFolderOrganizesReferencesToViews`. Extracted from the larger PR #4220 work — this PR contains only the source-generator View fix (no WriteMask, UserAccessLevel, NodeSet2.xml, or ReferenceNodeManager changes). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>	2026-08-14 07:57:06 -07:00
romanett	2c2bfa7080	Fix off-by-one ValueRank in the NodeState source generator (#4258) ... ## Summary `ModelDesignExtensions.GetValueRankAsCode` in the NodeState source generator computed the ValueRank for a fixed-size multi-dimensional `ArrayDimensions` string (e.g. `"3,3"`) as `dimensions.Length + 1`, which over-counted the rank by one. The number of comma-separated entries in `ArrayDimensions` already **is** the ValueRank — `"3,3"` is a two-dimensional array with ValueRank `2`, not `3`. ## Changes - `dims = dimensions.Length + 1` → `dims = dimensions.Length` (entry count already equals the ValueRank). - Added a guard: when there are zero entries (empty or all-commas `ArrayDimensions`, which `Split(..., RemoveEmptyEntries)` reduces to length 0), the method now returns `OneOrMoreDimensions`. - Refreshed the stale explanatory comment / removed the obsolete `TODO`. - Updated the `GetValueRankString_*` unit tests in `ModelDesignExtensionsTests.cs` to the corrected expected ranks. ## Validation - `dotnet build` of `Opc.Ua.SourceGeneration.Core` (Release): 0 warnings / 0 errors. - Filtered `GetValueRankString` tests on net8.0: 18/18 passed. Extracted from the larger PR #4220 work; this PR contains **only** the ValueRank fix (two files). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>	2026-08-14 07:56:23 -07:00
Marc Schier	e05a820c91	Removal of the exposed diagnostics and node locks (#4188) ... ## What this is Design analysis for `Opc.Ua.Server`, the two rounds of design exploration behind it, and the first two implementation chunks it recommends. Draft: the analyses are ready to read, but the *conclusions* deserve discussion before the larger phases proceed. Later chunks stack onto this PR as they land. | File | Kind | Lines | |---|---|---:| | `plans/NodeManagerAnalysis.md` | Evidence — the node-manager plugin surface | 488 | | `plans/ServerRuntimeAnalysis.md` | Evidence — the server runtime cluster | 576 | | `plans/ServerDesignExploration.md` | Design round 2 — delivery strategy | 1,394 | | `plans/ServerDesignExplorationAlt.md` | Design round 1 — four interface designs | 2,119 | | `src/Opc.Ua.Types/State/NodeState.cs` | **Code** — node self-guards attributes | | | `src/Opc.Ua.Types/State/BaseVariableState.cs` | **Code** — value accessors take the lock | | | `src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs` | **Code** — 6 external locks removed | | | `tests/…/NodeStateAttributeConcurrencyTests.cs` | **Code** — concurrency safety net | 449 | `docs/` is untouched — it holds shipped feature documentation, and none of this is shipped API. ## The analyses Counted mechanically over `src`, `samples` and `tests`; **every figure is reproducible** from commands printed in each document. **Node manager surface.** `AsyncCustomNodeManager` declares **93 `virtual`/`abstract` members** (80 distinct names). Across the repository: **112 override sites, 25 distinct names ever overridden, 55 of 80 never**. Six carry nearly all real use. For scale, `ReferenceNodeManager` is 6,373 lines and overrides **8**. **Server runtime.** Seven findings, **two of which recommend no change**. Findings 1–5 are filed as #4183–#4187. The headline is an `object`-typed sync root published on three public interfaces and taken in **88 `lock` statements across 7 files**, including a shipped sample. **What is deliberately not flagged.** `IStandardServer` — 9 members over 5,575 lines, 16 of 37 virtuals used — is identified as the reference shape to preserve. `IApplicationInstance` and the already-`[Obsolete]` `Stop()` are examined and left alone. ## The design exploration **Round 1** explored four alternative interfaces in parallel under the current back-compat constraint. Its value is the convergence: four designers who never saw each other's work independently arrived at a 6–9 member `IServerContext` replacing the 57-member locator, removal of the `object` node-manager handle, elimination of the `Processed` protocol, and `IHistorianProvider` as the pattern to copy. **Round 2** re-runs it with back-compat lifted and asks how to deliver: full rewrite versus side-by-side fork. **Both option designers independently declined to recommend their own assignment.** Three structural facts were verified: 1. **The fork line already sits at `IAsyncNodeManager`** — old and new node managers can coexist with zero runtime changes, so a fork duplicates ~10% of the assembly, not all of it. 2. **`[OpcUaShim]` cannot shim an inheritance surface** — honest auto-fix ceiling for the authoring surface is **~10%**, against ~70% for the shipped analyzers. 3. **A second lock leak exists** — `NodeState` takes `lock (this)` under a `CA2002` suppression whose own comment documents the contract. Recommendation: **neither pure option**, but a staged path reaching the rewrite end state incrementally, gated on migrating the five framework-internal node managers that today make it impossible for the old base to ever freeze. ## Implementation: NodeState now guards its own attributes This removes the prose-only contract in which `NodeState` took `lock(this)` and every external caller was required to take `lock(source)` on the node instance. The code already carried a TODO describing exactly this fix and noting it required updating every external site. - The node self-guards rather than relying on callers. `BaseVariableState` gains a **fully private** `System.Threading.Lock` covering its value fields; `NodeState`'s other collections keep their existing private locks. - All **6** `lock(this)` sites and both `CA2002`/`RCS1059` suppressions are gone. - `Value`, `Timestamp` and `StatusCode` on `BaseVariableState` take the lock too — without that, a direct property write raced an attribute read once the external lock was gone. - **6 of 9** external sites removed. Five were already redundant (`GetReferences`, `GetChildren`, `ReferenceExists` and `SetAreEventsMonitored` guard themselves; `FindMethod` reaches its state via `GetChildren`); the sixth was an attribute read now covered by the node's own lock. - Attribute read/write deliberately take **no** node-wide lock: both dispatch to caller-supplied handlers (`OnReadValue` and siblings) that may take locks of their own, and holding a lock on the node across one gives the node no fixed place in the lock order. - **3 deliberately retained and annotated in code**: two `CreateBrowser` calls, which are not internally synchronized (tracked in #4216), and one in `CustomNodeManager`, whose class is slated for deletion so refactoring its lock would be wasted work. ### No lock is exposed anywhere, to anyone An earlier revision put the attribute lock on `NodeState` as `private protected`, which still published a lock-sharing contract on the inheritance surface. Review pushed back and it is gone: `BaseVariableState` was the only user, so the lock moved there as a plain `private readonly Lock`. There is no member for a derived type to call, in this assembly or any other. `BaseVariableValue` was the last public leak — it handed out the lock guarding its value through a `public Lock` property, so any caller could hold it across a call back into the stack. That property is removed: - Derived value classes (what the source generator emits for every structure variable) synchronize through a `protected EnterLock()` / `ExitLock()` pair. Callback helpers were the first choice, but the generated bodies pass `ref Variant` / `ref StatusCode` / `ref DateTimeUtc`, and `ref` arguments cannot be captured by a lambda. The lock *object* stays private either way. - `NodeStateTemplates` emits the new form. - `ServerInternalData`, the one cross-assembly caller, was reading the lock back off a value it had itself constructed with that lock. It now takes `m_diagnosticsLock` directly. `NodeBrowser.DataLock` remains `protected` and is tracked in #4215: a subclass extending `Next()` legitimately needs its enumeration state atomic with the base browser's, so the fix needs a base-owned acquisition rather than a deletion. ### Migration coverage for the removed lock members None of the removed lock members get an `[Obsolete]` shim, and the migration guide now says why: a shim would have to be `object`-typed to keep old call sites compiling, and on .NET 9+ `lock (o)` on a statically-`object` reference is `Monitor.Enter`, which does **not** interlock with `Lock.Enter()`. It would compile, run, and silently synchronize nothing — strictly worse than a compile error. Re-adding a member to a downstream-implemented interface is also a break in its own right on `net48` / `netstandard2.0`, where there are no default interface methods. Instead, every removed member has a named replacement on the migration path: | Rule | Covers | Replacement it names | |---|---|---| | `UA0024` | `IServerInternal` / `ISession` / `ISubscription` `DiagnosticsLock`, `DiagnosticsWriteLock` | `UpdateServerDiagnostics(...)` / `UpdateDiagnostics(...)`, `ReadServerDiagnostics` / `ReadDiagnostics` | | `UA0025` | `ILocalNode.DataLock`, `Node.DataLock` | a lock the caller owns; the node guards its own state | | `UA0026` | `BaseVariableValue.Lock` | a lock passed to the `BaseVariableValue` constructor | Each rule fires on sources still written against 1.5.378, where the compiler only says a member is missing. `docs/MigrationGuide.md` gains a section per removal with before/after. ### The tests earned their keep The four concurrency tests were landed **first**, against the unmodified code, because removing a synchronization contract cannot be verified by the compiler and the existing `NodeStateHandlerConcurrencyTests` covers a different concern (racing a handler assignment against a write, not reader/writer exclusion). They now run with **no external locking**, which is what demonstrates the node guards itself. They caught two real problems, and one of them was mine: - The `Value`/`StatusCode`/`Timestamp` setters bypassed the new lock — a genuine race. - One test asserted an invariant the design does not offer. Two property setters are two lock acquisitions, so that pair was **never** atomic; it only passed originally because the test held a lock across both. It now uses a single attribute write carrying a whole `DataValue`, which genuinely is atomic and is the path the server uses. A third issue was found and fixed before it shipped: the reader loops gated on `!writer.IsCompleted`, so a reader scheduled after the writer finished performed zero reads — failing two runs in three on `net48`. Converted to `do…while`; now five consecutive green runs. **Verified:** `Stack.State` 116 passed on `net10.0` *and* `net48`; the concurrency category green 5/5 consecutive on `net48`; `Opc.Ua.Types.Tests` 8,456 passed; the server node-manager suite 1,205 passed, including `NodeManagerLifecycleTests` and `AsyncCustomNodeManagerTests` which exercise the changed paths. ## Two corrections to my own published figures Both were found while starting implementation, and both are recorded on #4183. **`ForceDiagnosticsScan()` was overstated.** The analysis claimed the `DiagnosticsWriteLock` getter *"triggers a full address-space diagnostics rescan"*. It is in fact a single flag assignment (`DiagnosticsNodeManager.cs:700-703`) consumed by the read paths and the periodic timer. The defect stands — a side-effecting property getter with a racy write on a synchronisation path — but its cost was misstated. **The contract was 15 sites, not 19.** A naive search matched ten comment and `#pragma` lines that *mention* `lock(source)` while explaining it. The documents now carry a counting command that excludes comment matches. A third finding expanded scope the other way: the `object`-typed lock leak is **repo-wide**, with ten such members across `Opc.Ua.Core`, `Opc.Ua.Types` and `Opc.Ua.Server` — including `Node.DataLock => this` and `ApplicationConfiguration.PropertiesLock => m_properties`, the same defect class as `Subscription.DiagnosticsLock`. ## Caveats stated in the documents - **Nothing in the designs is implemented or benchmarked.** Every performance claim across all seven designs is a design intention, not a measurement. - The larger phases are a **major undertaking against a live plugin API**; the documents say so rather than letting the neatness of the proposals imply otherwise. - The recommended sequencing puts low-risk, self-contained steps first and notes they are **worth doing regardless** of which strategy is eventually chosen. - Round 2 notes the two option designers' cost estimates are **not independent of their assignments**. ## Review guidance The two analyses make factual claims checkable by running the included commands. The design explorations are exploratory and need not be agreed with to merge. The code change is small, test-backed, and independent of which delivery strategy is eventually chosen. Reasonable outcomes include merging as-is, merging only the analyses and the lock fix, or disagreeing with the recommendation while keeping the evidence. --- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041 Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63	2026-08-10 06:47:50 +02:00
Marc Schier	2b44fcad22	Robot intent: a task-level command API with a Part 10 lifecycle (#4165) ... # Description Implement the draft **OPC UA — Robot Intent** companion model end to end: task-level motion verbs, a Part 10 lifecycle, a server hosting surface, a client, two samples and a live integration suite. It replaces the opt-in convention API merged in #4127 with an implementation generated from a companion information model. ## Why OPC 40010-1 *Robotics* describes robot **topology** in detail and defines **no motion verbs at all** — its entire actuation surface is `Start`, `Stop` and loading a named program. A conformant client can discover everything about a robot's construction and cannot ask it to move anywhere. #4127 filled that gap with ten verbs in an application-owned namespace, resolved by BrowseName, and the file said what it was: *"opt-in, explicitly non-normative industrial operation conventions"*. That contribution established a **vocabulary**. It established no **lifecycle**, and the lifecycle is the harder half. A motion takes seconds; a pick takes a minute. OPC 10000-4 §5.12.2 discards a method result when the Session ends *"independent of the task actually performed at the Server"* — so a synchronous motion method does not merely time out, it **loses the outcome of work that has already physically happened**. OPC 10000-10 §4.1 gives the OPC Foundation's own resolution: a Method performs a calculation, a **Program** runs a batch process or a machine tool part program. So `SubmitIntent` returns as soon as the intent is *admitted*, and what it returns is a NodeId — an `IntentOperationState` (a Part 10 program instance) the client subscribes to for progress and reads for the result. ## What changed **Model and shared code** — `Opc.Ua.RobotIntent.NodeSet2.xml` is source-generated in `Opc.Ua.Robotics`, so the enums, the polymorphic intent structures, `IntentOperationState : ProgramStateMachineState`, the method states and the typed clients derive from the model rather than being hand-written. Its only `RequiredModel` is the base UA namespace, so a server can adopt it without OPC 40010 or DI. `PoseMath` implements the specification's Annex C conversion between its unit quaternion and the core `ThreeDFrame` — including the `asin` clamp that stops a pole orientation becoming a domain error, and the non-negative-`w` representative that makes two servers agree on four numbers. `FrameTree` composes transforms along the frame tree. **Verbs are a DataType hierarchy, not one Method each.** A single submission and a mission step are then the same shape, and a new intent is a subtype rather than a new method — which is what `AddOperation<TRequest,TResponse>` was working around. **Server** — `IntentControllerHost` owns admission in the specification's order, the queue with PLCopen buffer modes, blending, cancellation *with the server's right to refuse*, missions with a committed base and a revisable horizon plus an IEC 61131-3 step graph, safety-aware refusals, real-time channel brokerage, and the capability declaration. All thirteen Methods are wired; command authority and channel leases are released when the holding Session closes; every client-supplied NodeId is validated against the controller being commanded (clause 11.3). `RobotIntentNodeManager`, the fluent `IIntentControllerBuilder` and `AddRobotIntent`/`ConfigureRobotIntent` make it reachable, and `RobotIntentFacetCalculator` computes which of the specification's facets an instance actually satisfies. **Client** — `RobotIntentClient` discovers `Server/RobotIntent/Controllers` and reads what a robot accepts; `IntentOperationHandle` subscribes and completes on a terminal state, recovering the result after a reconnect because clause 6.7 promises it survives; `CommandAuthorityLease` and `RealTimeChannelLease` manage the two exclusive resources; fluent builders cover every intent and mission. **Samples** — all robotics samples move under `samples/Robotics/`. `IntentEnabledRobot` is one UR5e-style offset-wrist arm on a bench, declared completely enough to show what a conformant server publishes, with a simulated safety source so the clause 10.4 refusals are demonstrable. `IntentViewerClient` turns a **click on a target prim in the OpenUSD viewport** into an intent and watches the arm execute it; it also runs headless, which is how CI exercises it. **Intents execute serially**, which satisfies every `BlockingMode` constraint by construction: the specification forbids beginning a `Single` or `Hard` intent while another executes and merely *permits* `None` and `Soft` to overlap. The convention API (`RoboticsOperationConventions`, `RoboticsOperationsBuilders`, `RoboticsOperationsClient`) is removed. ### On safety — what this deliberately does *not* claim The interface is **non-safety-rated**, and that is a property of the technology rather than a scoping choice. OPC 10000-15 carries cyclic safety data from a SafetyProvider to a SafetyConsumer; the consumer's `RequestSPDU` holds an identifier, a monitoring number and one octet of explicitly **non-safety** flags, so a caller has no channel through which to supply safety-rated arguments. Every safety fieldbus (PROFIsafe, CIP Safety, FSoE, openSAFETY) expresses a safety command as a **continuously asserted cyclic signal**, because the integrity argument rests on the fail-safe state that follows when assertion *stops* — and a Method call has no defined behaviour when it stops being called. What the host does instead is **observe and refuse**: `UpdateSafetyState` is how the application reports what the safety system is enforcing, and admission then refuses on the same values a client can read, so a refusal is explainable from the address space rather than from Server-internal state. It may never *instruct* the safety system, and no Method here commands a safe motion function, changes an operational mode or clears a stop. ## Defects this found in the specification, fixed upstream Implementing a specification is the only way to find out whether it can be implemented. Seven defects went back to [marcschier/opcua-drafts#47](https://github.com/marcschier/opcua-drafts/pull/47) rather than being worked around here: - **Refusal was unobservable.** Clause 6.2 orders six distinct refusals and clause 5.8 calls the failure set "small and diagnosable on purpose" — but `SubmitIntent` returned only `IntentId` and `Operation`, so a client could see *that* something was refused and never *which*. It now returns `Accepted`, `Failure` and `Message`, and clause 6.2 states that a refusal returns `Good` rather than substituting a Bad `StatusCode`. - **The Part 10 promotion was illegal.** `IntentOperationType.ProgramDiagnostic` was declared as a Property of `PropertyType`; OPC 10000-10 declares it a Variable of `ProgramDiagnostic2Type` reached by `HasComponent`. The declaration added a *second* member beside the inherited one instead of promoting it — which is precisely why an earlier version of this branch had deleted it. Correcting the TypeDefinition made it generate cleanly with **no change to the curated Core**. - **Clause 9 said nothing about the Method surface.** A server could declare `MissionsSupported` true and omit `SubmitMission` entirely, because the mission and channel Methods are Optional. This implementation did exactly that until an integration test caught it. A fourth honesty rule and a table now fix which Methods each declaration implies. - **Conformance was not machine-readable, and two implementations disagreed.** Clause 12 defined conformance in terms of facets and gave a Server nowhere to publish which ones it satisfied, so every client had to re-derive Table 12.2 from the address space. This repository proved why that fails: the server''s `RobotIntentFacetCalculator` and the client''s `RobotIntentRules.DeriveFacets` each implemented clause 12.2 independently and **disagreed on essentially every row** — the client''s `Blending` was `BlendingSupported` alone where the server also requires all four blending buffer modes, its `QueuedIntents` was `MaxQueueDepth > 0` where the server also requires `Buffered`. A client and server from the same release answered "does this controller conform?" differently, and both were reading the specification correctly, because several rows are behavioural and no amount of browsing settles them. `IntentCapabilitiesType` now carries **`SupportedFacets`**, exactly as OPC 10000-5 does with `ServerCapabilitiesType.ServerProfileArray`, RI-Base requires it, and clause 12.2 separates structural requirements a Server shall meet before listing a facet from attested ones governed by clause 9. - `WaitIntentDataType.Signal` was unbounded where clause 11.3 has to check it; §11.3 gained a table fixing the expected type of every NodeId-valued member; §5.7.0 gave `Ready`/`ActiveIntent`/`ActiveMission` normative meaning; §6.9 bounded `RequestedLease`; §6.5 and §6.4 now say what a server that cannot differentiate `StopMode` must do, and which stop a superseded intent gets. ## Bugs the tests and reviews caught Each of these would have been silent in the field: - **The execution engine was never wired into the public API.** `IntentControllerBuilder.RegisterAsync` built the address space and never started an `IntentControllerHost`, so through the documented `AddRobotIntent` → `ConfigureRobotIntent` path **every Method returned `BadNotImplemented`** and the registered `IIntentExecutor` was never resolved. A browsable address space is not a working robot interface, and only a live integration test showed the difference. - **NodeId scoping failed open on an empty index.** A controller advertising `CallProgramIntentDataType` with no published `ProgramType` accepted an *arbitrary* NodeId as `CallProgram.Program` — and `CallProgram` runs code the server holds. - **Blending completed the predecessor at submission time** and abandoned its motion, telling the client it succeeded and publishing an all-zero pose at the origin as a genuine `AchievedPose`. - **Mission base immutability compared only identifiers**, so a client could keep the ids and swap the *motion* of an already-committed step. - **`FinalResultData/Result` was never registered** with the node manager: Browse showed it, Read returned `BadNodeIdUnknown` — so clause 6.7's promise to a Part 10 client did not hold. - `DisposeAsync` disposed primitives out from under in-flight executor code; re-using a terminated `IntentId` destroyed the retained result and duplicated a NodeId; neither side registered the Robot Intent encodeables, so `Pose3DDataType` did not decode; host startup was hooked to a task that ran in only one hosting shape. - The result was published **after** the state went terminal; capabilities were resolved against a stale namespace table; an empty `ContentFilter` arrives as an empty element array rather than a null filter, so testing only for null made every unconditional mission transition silently untaken — a mission would run its first step and stop. ### Found by a later test-quality audit, after the above was written An audit of the test suite itself found five tests whose **names** asserted a normative requirement while their **code** asserted something weaker or nothing at all. The worst **fabricated the answer it checked**: a helper caught a decode failure and invented "5 supported intents, 6 axes, all facets true", so six capability-honesty assertions across four tests could pass while reading nothing from the server. The fallback is gone and the suite passes with honest reads. Chasing the facet gaps it reported led to the clause 12 defect above. Two further defects followed from completing that work: - **`RI-Interop-40010` was never computed**, and the code documented the omission as impossible to fix "because the calculator receives only a Robot Intent controller, not the linked OPC 40010 model". That was wrong: Annex B's structural requirement is an **inverse** `HasIntentController` reference, reachable from `IntentControllerState` — and the integration suite was already browsing exactly that. A server implementing Annex B could not claim Annex B. - **The published claim could go stale.** `SupportedFacets` was a snapshot taken during `RegisterAsync`, so a reference attached afterwards left it wrong. It is now bound through `BindRead` and recomputed on read, so the claim tracks the address space by construction rather than by discipline — which matters for anyone bolting Robot Intent onto an existing OPC 40010 node manager, where the attach ordering is often not theirs to choose. Two tests that **could not fail** were also replaced: one asserted `AccessLevel` equals `CurrentRead` to pin a read-only marking, but `BaseVariableState` initialises every variable to exactly that, so deleting the marking left it green; it is now a write expecting `BadNotWritable`, including a real OPC UA `Write` service call over a live session. Finally, a build break that a plain build hides: `RoboticsAotTests` used `out string?` in a project declaring `<Nullable>disable</Nullable>`. Building the project **without** `-f` suppresses `CS8632` through `NoWarn=nullable` and succeeds; building it **with** `-f net10.0` resolves `NoWarn` differently, `CS8632` fires, and `TreatWarningsAsErrors` makes it an error. CI builds per-TFM, so this would have failed there while passing every local build to that point. ## Testing and coverage | | | |---|---| | `dotnet build UA.slnx -t:Rebuild` | **0 errors, 2 warnings** — both pre-existing `CA1861` in `tests/Opc.Ua.Tools.Tests`, a file this PR does not touch | | **Whole `UA.slnx` test suite, net10.0** | **50 of 52 assemblies fully green** (measured at `40d9cf213`, before the `master` merge; the other two are explained below) | | `Opc.Ua.Robotics.Tests` | **467** net10.0, **447** net48 | | `Opc.Ua.Robotics.Intent.Tests` (live client <-> server) | **20** on net10.0 and net48, run repeatedly | | `Opc.Ua.OpenUsd.Tests` | **904** after merging `master` (664 before; the rest arrive with #4160 / #4162 / #4180) | | `Opc.Ua.Aot.Tests` | **121/121** | | `Opc.Ua.Di.Tests` (regression from the sample move) | **364/365**, one pre-existing skip | | **Changed-line coverage** | **90.7 %**, no file below 80 % — comfortably above the enforced band, which after #4158 applies only to patches larger than 100 changed lines | The two non-green assemblies are **not** caused by this branch. Rather than assert that, I ran the same selections at the merge-base `ae241b662` on the same machine: - `ServiceCallReassemblerTests.PrivateRequestSummaryFormatsKnownRequestKinds` expects `12.5` and gets `12,5` — a culture-sensitive assertion failing on a European locale. Fails 4/4 on **both** commits. - `CertRotationLiveTests.L2Cert2RotateCertificateDuringServerRestartRecoversSharedChannelAsync` fails on a teardown count. Fails 4/4 on **both** commits. - A third failure seen once, AOT `FindServersAsync` with `BadSecureChannelClosed`, did not reproduce: 4/4 on retest and 121/121 in the final run. Environmental. Two things are worth calling out because they would otherwise be invisible: - `tools/**` is **not** in the `coverage-thresholds.json` ignore list, and no test project referenced the OpenUSD tools assemblies — so their changed lines counted as uncovered and the real patch figure was 75 %, exactly the blocking threshold. Rather than widening the ignore list, the pure decisions in the viewport host (pointer-to-physical-pixel conversion, `RenderPickRequest` construction, stale-retry, backend discovery, pick-mode selection) were separated from the Avalonia/native shell and tested headless against a fake picking backend. - `dotnet build UA.slnx` **hides warnings on unchanged projects**; only `-t:Rebuild` surfaces them. That is how a `CA2263` introduced by a net48 portability fix survived several "zero warning" builds. The multi-target requirement earned its keep repeatedly: it caught a default interface implementation, `ArgumentNullException.ThrowIfNull`, `string.Create` with an interpolated handler, `ValueTask.FromResult`/`CompletedTask`, `Task.WaitAsync`, generic `Enum.GetValues` and generic `Enum.IsDefined`, and a non-generic `TaskCompletionSource` — none of which the net10.0 build complained about. A net10.0-**only** hang in the simulated executor and two load-sensitive flaky tests were also found and fixed at the cause. ### Merged with `master` `master` (through `e73e71184`) is merged in as `08205d81d`. Four conflicts, all where the two branches touched the same robot and OpenUSD surfaces, and none resolved by dropping a side: - #4162 added six files to `samples/MinimalRobotServer/` while this branch moved that directory to `samples/Robotics/MinimalRobotServer/`. They are kept at the moved path, and `UA.slnx` takes the union of both sides' new projects. - #4160 replaced `object` with a `UsdValue` union and rewrote `Flatten`; this branch had reformatted the same method and fixed a path in its doc comment. Git produced a hybrid that did not compile. The resolution keeps `master`'s `UsdValue` arm and `Flatten(UsdValue)` overload in this branch's block-bodied form, with the path fix retained. - `master` inserted `FindStageCamera` directly before two methods this branch had relocated, so the merge duplicated `WriteStageUsda` and `GetPrivateStateRoot`. Only `FindStageCamera` is genuinely new — `master`'s diff shows the other two as unmodified context — so the duplicates are dropped and this branch's `internal` versions kept, including the `GetPrivateStateRoot(string?)` overload the connector tests use to stay out of the real user profile. - `master` moved the OpenUSD stack to `0.4.0-alpha`; this branch pins `OpenUsd.Rendering` in the test project so the renderer-pick tests can reach the picking backend. Left at `0.1.0-alpha` that pin is below what the viewer now resolves transitively, which is `CS1705`. The pin follows `master`. CI was green on `baaf35e9` with **190 checks passing and none failing**, across every build leg (net472 / net48 / netstandard2.0 / netstandard2.1 / net8.0 / net9.0 / net10.0, Debug and Release) and the full test matrix on Windows and Linux. Since then `master` has been merged twice more and two CI-stability fixes landed (see below), so the current head is re-running. Locally, `dotnet build UA.slnx -t:Rebuild` gives **0 errors** and the 2 pre-existing `CA1861` warnings; Robotics **467** net10.0 / **447** net48, Robot Intent **20** on both, OpenUSD **904**, AOT **121/121**, source generation **3771**. ### Review round Five reviewers over disjoint areas found defects worth calling out, all fixed in `3ffec2c1`: - **A refused mission step was reported `Succeeded`.** The advance path returned the same `false` for "steps exhausted" and "step refused", so a protective stop between steps told a supervisory system the mission had completed while the robot stood halted with work unexecuted. - **Three attacker-controlled NodeIds reached the executor unvalidated** — `Pick.Tool`, `Place.Tool`, `ToolChange.DockStation`. `DockStation` drives a physical tool-exchange motion to a location the server never authorised. The replacement test enumerates NodeId members by reflection so the next one added cannot be missed. - **A lock-order inversion that loses a stop.** `CanCancel` and the mission condition evaluator ran under the host lock while the executor's own thread re-entered it via `ReportBlendBegin`. - **A non-terminal executor outcome killed the pump** from outside the `try/catch` that exists because the executor is untrusted — `Ready` stayed true and submissions kept being accepted while nothing moved. - **`Pause` published `Suspended` while the robot kept moving.** Now queue-only and honest about it; the specification gained the matching rule ([opcua-drafts `357a558`](https://github.com/marcschier/opcua-drafts/pull/47)). - **`RI-RealTimeChannel` and `RI-Safety` were false claims** — granted on an empty folder with no methods, and on the mere presence of a Mandatory node. - **The Part 10 promotions never promoted.** Namespace-1 BrowseNames declared second members beside the inherited ones; the conformance test could not see it because its lookup stripped the prefix. Fixed upstream and regenerated. - **Renderer picking never worked.** The reflective probe bound a renderer that is not a picking backend and throws off its owner thread. It now binds only `IRenderPickingBackend`, falls back immediately and loudly, and the docs say plainly that renderer picking is unavailable with this package version. - **The client lost command authority the instant it gained it** — monitored items report the current value first, and the lease treated every notification as a loss. ### CI stability fixes for master-side failures The CI split in #4158 turned on legs that had never run, and two of the failures they exposed were real bugs on `master` rather than anything this branch introduced. Both are fixed here because they were blocking every branch: - **ECC key generation was broken on macOS.** `ExportExplicitParameters` throws `PlatformNotSupportedException` there — Apple's SecurityTransforms supports named curves only — and it was called for exactly one value, the curve order `n`. The four curves OPC 10000-12 admits have published orders, so those are carried directly, with `CurveOrderTableMatchesThePlatform` checking each against the platform's own exported order wherever export works, so the constants cannot silently drift. This accounts for the two ECC failures in `test-macOS-latest-Server`. - **The certificate-leak detector raced asynchronous disposal.** It polled a fixed budget and reported whatever it saw, which cannot distinguish a genuine leak (count steady) from a suite still draining fire-and-forget disposals (count falling) — on a loaded agent that produced `Certificate leak detected: 2 instance(s)` with every test passing. It now waits for the count to **settle**, so a real leak is reported sooner than before while a slow drain gets as long as it keeps progressing. Shared by every suite that asserts no certificate leaks. The remaining macOS failures on `master` (`Gds`, `Tools`, `Redundancy.Samples`, `aot-macos`) have different root causes, are not reproducible without a macOS host, and are **not** addressed here. ### Merged with `master` again `master` through `416d619c4` (#4158, the CI matrix split and graduated coverage bands) is merged in as `08ae4528`. One conflict, in `docs/DeveloperGuide.md`, where both sides had deliberately rewritten the coverage-gate section: - `master`'s rewrite is kept in full — graduated `patch.bands`, Codecov being informational-only, and where the numbers surface per CI system. - This branch's paragraph about the `ignore` globs is kept too, re-sited ahead of those subsections. I re-checked it against the merged `coverage-thresholds.json` rather than assuming it still held: the list is still `tests/**`, `samples/**`, `obj`, `bin`, `*.g.cs`, with **`tools/**` absent** — so the warning that anything under `tools/` is measured like product code, and that an assembly no test project references contributes changed lines counted as *uncovered*, is still accurate and worth keeping. - `master`'s "reproduce a coverage failure locally" line supersedes this branch's near-identical one, so only `master`'s survives. Post-merge: `dotnet build UA.slnx -t:Rebuild` **0 errors**, 2 pre-existing `CA1861`; Robotics **467** net10.0 / **447** net48; Robot Intent **20** on both. ### Getting CI green The pipeline had never actually run on this branch until now (it was waiting on the fork-PR approval gate), so the first run surfaced four real problems, each fixed at source: - **Seven build legs failed** because `IntentEnabledRobot` pins `net10.0` for the OpenUSD payload but never opted into `RestrictForLegacyTfm`. On the legacy legs the libraries build as net472/net48, where `Opc.Ua.Types` exposes the `System.Threading.Lock` polyfill publicly, and that collides with the sample's own net10.0 BCL (`CS0433`). `Opc.Ua.OpenUsd.Connector.Viewer` was already using the opt-out for the same reason. - **A Linux-only test failure** turned out to be a real defect, not a platform quirk: `SetPose` published the *commanded* interpolated pose beside joint frames derived from the *solved* joints, so the two disagreed by the inverse-kinematics residual — about 8 mm, constant after any real motion. That is the lying-digital-twin defect one level deeper than the per-step IK fix reached. `SetPose` now derives both from the joint angles. - **Three source-generator tests** asserted the old emitted shape. The clause 6.2 Null-Variant fix was implemented in `ObjectTypeProxyGenerator` so every generated proxy benefits, which changed that shape; the tests are updated and strengthened to pin the `IsNull` branch as well. - **A CI-only timeout** in the simulated arm drain: the executor runs on a manual clock so the tests do not depend on machine speed, but the drain guarded itself with a wall-clock deadline, which put that dependence back. CI runs with coverage instrumentation, which is enough to take a per-step IK probe from ~0.6 s to over 5 s. The guard is now a simulated step budget. One failure was **not** from this PR and did not recur: a Roslyn `AccessViolationException` compiling `Opc.Ua.ISA95` on one leg, while the Debug leg of the same target passed on identical code and the Release build succeeds locally. ## Related Issues - Relates to #3827 — the request that prompted the robotics intent work. - Builds on #4127. The companion specification is drafted in the open at [marcschier/opcua-drafts#47](https://github.com/marcschier/opcua-drafts/pull/47) (`metaverse-specs/robot-intent/`), with the prior art and the reasoning behind each decision in the research document beside it. **Nothing in it is normative or endorsed by the OPC Foundation, and its NodeIds and namespace URI are provisional** — which is the main reason this is a draft. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. On the test checkbox, precisely: the whole `UA.slnx` suite now runs on net10.0 with **50 of 52 assemblies fully green**, and `Opc.Ua.Robotics.Tests` passes on **all five** target frameworks including net472, net8.0 and net9.0 which had never previously been exercised. The two remaining failures are reproduced identically at the merge-base and are detailed above, so I have ticked the box; if you would rather it stayed unticked until those two are green, say so and I will revert it. ## Points I would most like feedback on 1. **The namespace and NodeIds are provisional.** They come from a draft specification, not from the OPC Foundation. If this lands before the model is registered, the identifiers will move. 2. **The model lives in `Opc.Ua.Robotics`.** It is standalone on the base UA namespace and takes no dependency on OPC 40010, so a separate `Opc.Ua.RobotIntent` package would arguably be cleaner. Keeping it here was a deliberate call — this is where the robot-facing API already is — but it is the easiest thing to change now and the hardest later. 3. **Removing the convention API from #4127 is a breaking change** for anyone who adopted it. It was explicitly marked non-normative, so I have assumed that is acceptable — please say if a deprecation period is wanted instead. 4. **Serial execution** is a deliberate simplification. Concurrent `None`/`Soft` intents would be an optimisation, not a correction, but it is worth agreeing that reading. 5. **`samples/Robotics/` reorganisation** moves `MinimalRobotServer`. Happy to split that into its own commit if it makes review easier. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b	2026-08-08 09:08:40 +02:00
Marc Schier	e73e71184d	[WotCon] Add OPC UA WoT Connectivity 1.1 with the Types, source generation, Server and xRegistry work it builds on (#4128) ... ## Summary Adds OPC UA WoT Connectivity 1.1 to the stack, together with the Types, source generation, Server and xRegistry work it is built on. This branch is the merge point for the fourteen-PR stack that replaced integration PR #4093; every constituent PR was reviewed and approved separately and is listed below. 414 files changed, +108,936 / -3,199. Roughly 46k added lines of product code, 57k of tests, plus samples, tools and documentation. ## Types - Complete `ExtensionObject` raw-body decoding for binary, XML and JSON bodies by resolving the concrete type through the message context's encodeable factory, and simplify the `Variant` structure helpers by delegating to it (#4128). - Add `IAtomicFileReplace` as an optional `IFileSystem` capability so existing external implementations keep working, with atomic publish for `LocalFileSystem` via `File.Replace`/`File.Move` and for `VirtualFileSystem` by re-keying the in-memory entry (#4128). - Add lossless conversion between WoT documents and NodeSet2 (`Opc.Ua.Wot.WotNodeSetConverter`). A byte-exact `uav:nodeSet` envelope is preserved when requested, the structured `uav:nodes` projection is used when the readable vocabulary is incomplete, and NodeSet2 is otherwise synthesized from readable WoT terms. Unmapped JSON members survive a round trip as pointer-addressed residue (#4131). ## Source generation - Improve the NodeSet to ModelDesign conversion and the generated node state (#4132). - Generate OPC UA models directly from WoT Thing Description files (#4134). ## Server - Add live NodeManager shadow and immediate reload. A reload materializes a new generation beside the active one and switches atomically; the superseded generation is retired gracefully so existing MonitoredItems keep being served until they drain, or immediately when the caller asks for it. Includes the request admission and drain machinery that lets an orderly shutdown wait for admitted requests instead of tearing down underneath them (#4147). - Make subscription transfer between sessions transactional, so a failed transfer leaves neither session holding a partially moved subscription (#4150). ## xRegistry - Allow clients to use explicit registry roots rather than assuming a well-known location (#4130). ## WoT Connectivity - Add the WoT Connectivity 1.1 information model: a registry-first revision layered on the abstract xRegistry base model, incorporating the complete published OPC 10100-1 v1.02 surface as deprecated nodes in the same namespace so existing 1.02 clients keep working (#4139). - Add the protocol binding abstractions and planners that compile WoT forms into executable plans (#4140). - Add the HTTP, Modbus, OPC UA and MQTT binding executors (#4144). - Add the registry and its stores, including document versioning, validation, dependency resolution and refresh (#4146). - Add the registry client (#4142). - Materialize WoT Thing Descriptions into the server address space, deriving types from Thing Models and instances from Thing Descriptions (#4154). - Restore the WoT asset registry bridge so the deprecated 1.02 asset surface is backed by the registry (#4171). - Add the WoT Connectivity samples and complete the documentation (#4156), and restore the remaining coverage and docs (#4172). ## New projects `src/Opc.Ua.WotCon.Bindings`, `src/Opc.Ua.WotCon.Bindings.Mqtt`, `tests/Opc.Ua.WotCon.Bindings.Tests`, `tests/Opc.Ua.WotCon.Samples.Tests`, and the `samples/WotCon` AggregationServer, AggregationClient and FlatTagServer samples. ## Constituent pull requests Merged into this branch top-down, each independently approved: | PR | Title | | --- | --- | | #4128 | [Types] Complete ExtensionObject decoding and add an atomic file replace capability | | #4131 | [Types] Add lossless conversion between WoT documents and NodeSet2 | | #4132 | [SourceGeneration] Improve NodeSet to ModelDesign conversion and generated node state | | #4134 | [SourceGeneration] Generate OPC UA models from WoT Thing Description files | | #4147 | [Server] Add live NodeManager shadow and immediate reload | | #4150 | [Server] Make subscription transfer between sessions transactional | | #4130 | [XRegistry] Allow clients to use explicit registry roots | | #4139 | [WotCon] Add the WoT Connectivity 1.1 information model | | #4140 | [WotCon] Add the WoT protocol binding abstractions and planners | | #4144 | [WotCon] Add the HTTP, Modbus, OPC UA and MQTT binding executors | | #4146 | [WotCon] Add the WoT Connectivity registry and its stores | | #4142 | [WotCon] Add the WoT Connectivity registry client | | #4154 | [WotCon] Materialize WoT Thing Descriptions into the server address space | | #4156 | [WotCon] Add the WoT Connectivity samples and complete the documentation | | #4171 | Restore WoT asset registry bridge | | #4172 | Restore WoT coverage and docs | Supersedes #4093. ## Follow-up The WoT Connectivity and WoT Binding drafts have moved to 1.1-draft2 since this work was authored. Aligning with that revision - the removed group vocabulary, the new projection/View construct, the remaining model vocabulary terms and the two new portable-identity validation rules - is tracked separately and will follow in its own pull request. ## Validation Built for every target framework with zero warnings and zero errors, and validated at the tip of the stack with `Opc.Ua.WotCon.Tests` at 999 passed / 0 failed and `Opc.Ua.Server.Tests` at 4024 passed / 0 failed. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8	2026-08-05 10:14:15 +02:00
Marc Schier	00664404c0	Ship the OPC Foundation license file from every package and fix the NU5033 pack failure (#4174) ... # Description The `preview-pack` stage of the Azure DevOps pipeline fails with nine `NU5033` errors, which fails the whole build: ``` NuGet.Build.Tasks.Pack.targets(222,5): error NU5033: Invalid metadata. Cannot specify both a PackageLicenseExpression and a PackageLicenseFile. [src\Opc.Ua.Robotics\Opc.Ua.Robotics.csproj] ``` `common.props` gives every package the shared `LICENSE.txt` as its `<PackageLicenseFile>`, but **19** packages also declared `<PackageLicenseExpression>MIT</PackageLicenseExpression>`. Nine of them hit `NU5033`; the other ten only avoided it by carrying an empty `<PackageLicenseFile></PackageLicenseFile>` workaround. That declaration was wrong on two counts. **It advertised the wrong license.** `LICENSE.txt` is the **OPC Foundation MIT License 1.00**, which carries the OPC Foundation copyright and preamble — it is a different text than the plain SPDX `MIT` license. The affected packages therefore published `<license type="expression">MIT</license>` plus a `licenses.nuget.org/MIT` URL on nuget.org, instead of the OPC Foundation text that every other package in the stack ships. **It also broke the build,** because NuGet refuses to pack a project carrying both a license expression and a license file, failing `pack` solution-wide rather than just for the offending projects. ## Changes The expression is dropped from all 19 packages so every one uses the shared OPC Foundation license file uniformly, like the rest of the stack: * `src/Opc.Ua.OpenUsd`, `.Client`, `.Server` * `src/Opc.Ua.OpenUsdScene`, `.Server` * `src/Opc.Ua.Robotics`, `.Client`, `.Server` * `src/Opc.Ua.Positioning`, `.Client`, `.Server` * `src/Opc.Ua.Di`, `src/Opc.Ua.ISA95`, `src/Opc.Ua.WotCon`, `src/Opc.Ua.Gds.Common` * `tools/Opc.Ua.OpenUsd.Connector.Viewer`, `tools/Opc.Ua.SourceGeneration.Core` * `tools/Opc.Ua.SourceGeneration.Pack`, `tools/Opc.Ua.SourceGeneration.Stack.Pack` — these two set the license not in their own `.csproj` but in the shared `tools/SourceGeneratorPack.targets`, so they were only caught once the new guard ran in CI. The eight per-project empty `<PackageLicenseFile></PackageLicenseFile>` workarounds go away with it. To stop the whole class of failure from recurring, `Directory.Build.targets` now *enforces* the rule rather than papering over it. Silently clearing the inherited license file would have fixed the build while still publishing the wrong license metadata, so instead a violation fails fast with an actionable error naming the project: ``` error OPCUA0001: Opc.Ua.Foo sets PackageLicenseExpression=MIT. Every package must ship the OPC Foundation MIT License 1.00 via the shared LICENSE.txt that common.props sets as PackageLicenseFile, which the SPDX expression neither matches nor is allowed to coexist with (NU5033). Remove the PackageLicenseExpression property. ``` `common.props` cannot catch this itself: it is imported from `Directory.Build.props`, i.e. **before** the project body, so it never observes a project-level license expression. `Directory.Build.targets` is imported **after** the project body and is the earliest place that can. The `common.props` comment that previously hinted the expression was an option now points at this rule. ### Verification * Every packable project evaluates to `PackageLicenseFile=LICENSE.txt` with an empty `PackageLicenseExpression`; file-licensed packages such as `Opc.Ua.Client` are unchanged. * `dotnet pack -c Release` succeeds for `Opc.Ua.Robotics` (previously failing) and `Opc.Ua.SourceGeneration.Core` (previously relying on the removed workaround). * Replicated the CI build commands locally — `dotnet build` of `UA.slnx` (net48/Debug), `tools/SourceGeneration.slnx` (net472/Release) and `tools/MigrationAnalyzer.slnx` (net48/Debug) all succeed with 0 errors. * Inspected the produced `.nupkg`: now `<license type="file">LICENSE.txt</license>` with the license text at the package root, matching the rest of the stack (it was `<license type="expression">MIT</license>` before). * Confirmed the guard fires on a simulated violation (`-p:PackageLicenseExpression=MIT`) and is inert otherwise. ## Related Issues No tracking issue — this fixes the `NU5033` failure in the OPC Foundation Azure DevOps pipeline (build 16247, `preview-pack` stage) and corrects the license metadata published for the affected packages. No product code or public API changes. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. _(Build-infrastructure change with no code surface. Validated by `dotnet pack` of the affected projects, by inspecting the resulting package metadata, and by confirming the new `OPCUA0001` guard fires on a simulated violation. The pipeline `preview-pack` stage is the regression test.)_ - [x] I have added all necessary documentation. _(The rule is documented in place, on the new `Directory.Build.targets` guard and at the `PackageLicenseFile` definition in `common.props`.)_ - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received.	2026-08-04 18:49:50 +00:00
Marc Schier	724ae71f8a	Make NodeId assignment during a node copy explicit (#4157) ... Fixes #4137. ## The problem `NodeState.Initialize(ISystemContext, NodeState)` copies a node. Every child it materialises through `CreateChild` gets a per-instance NodeId from `ISystemContext.NodeIdFactory`, and is then initialised from its source on the very next statement - which overwrites that NodeId. The assignment is therefore *always* discarded: it only burns identifiers, and permanently leaks them for factories that track outstanding allocations. Booting the Robotics node manager left ten reservations behind that every later allocation skipped. #4123 fixed the symptom by hiding the factory for the duration of the copy, wrapping the context in one that reports `NodeIdFactory` as `null`. The review objection was fair: a context that lies about one property is hard to reason about, and it only works because that property is allowed to be null. ## What this does States the intent instead of faking the environment. NodeId assignment is now an argument of the two members that materialise children: ```csharp public virtual BaseInstanceState? CreateChild( ISystemContext context, QualifiedName browseName, bool assignInstanceNodeIds = true); protected virtual BaseInstanceState? FindChild( ISystemContext context, QualifiedName browseName, bool createOrReplace, BaseInstanceState? replacement, bool assignInstanceNodeIds = true); ``` A copy passes `assignInstanceNodeIds: false` and every node type - generated or hand-written - sees the real context. There is no capability property, no wrapper, and no second overload to keep in sync. `MethodState`, `BaseDataVariableState` and the source generator template each collapse to a single `FindChild` override that threads the argument into its `CreateOrReplace<Child>` helpers. ## Breaking change The four argument `FindChild` and the two argument `CreateChild` virtuals are gone. - **Call sites are unaffected.** The new parameter defaults to `true`, so `CreateChild(context, browseName)` and `FindChild(context, browseName, true, null)` keep compiling and keep the 1.5.378 behaviour. - **Overrides must add the parameter.** A node manager that overrides either member gets `CS0115` until it does. The compiler points at every site; `docs/migrate/2.0.x/node-states.md` shows the before/after. This is deliberate, per review: the stack ships all base types, and types generated from a model are regenerated for 2.0 anyway. ## Also fixed `MethodState.FindChild` returned `OutputArguments` when asked for `InputArguments` with `createOrReplace: false` - a pre-existing bug in the method being restructured. Covered by `FindChildReturnsTheRequestedArgumentsProperty`. ## Validation - Full `dotnet build UA.slnx`: **0 errors, 0 warnings**. - `Opc.Ua.Types.Tests`, `Opc.Ua.SourceGeneration.Core.Tests`, `Opc.Ua.SourceGeneration.Tests`, `Opc.Ua.Di.Tests`, `Opc.Ua.Robotics.Tests` - all passing. - Each new regression test was checked to fail without its corresponding fix. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485 Copilot-Session: f45109d9-59cb-4ffd-a4f7-16e1ea449ae4	2026-08-03 20:36:32 +00:00
Marc Schier	0357380335	Source generator: typed NodeSet method signatures and referenced model accessors (#4124) ... **Stack 2/5** — base #4123. Part of the #4096 split; see #4096 for the overall picture. > Review after #4123. The diff shown against `master` will include #4123 until that merges. ## What this is Source generator only; no runtime library changes. ## Contents - **Typed NodeSet method signatures.** A method imported from a NodeSet now gets a typed signature. Methods that share a declared signature are grouped onto one generated `MethodType`, named after its owner when the plain name would collide, so two unrelated `Execute` methods no longer share a class. - **An override that keeps the signature it inherits keeps the declaration it overrides.** Pointing `MethodDeclarationNode` at the override itself made `MethodDeclarationId` self-referential. OPC 10000-4 lets a caller invoke a method with the NodeId of its declaration in the ObjectType *or any supertype*, and `MethodState.MethodDeclarationId` is what the server matches on, so `Call` stopped resolving and returned `Bad_MethodInvalid` for every alarm subtype. An override that genuinely changes its signature still becomes its own declaration. - **`StateNumbers` / `TransitionNumbers` were emitted empty for every NodeSet-sourced model**, because such children arrive as `VariableDesign` rather than `PropertyDesign` and were filtered out. Any companion model built from a NodeSet was writing `StateNumber = 0` on the wire — a Part 16 defect well beyond the models in this stack. The ModelDesign path is unchanged. - **Referenced models can contribute fluent accessors**, carried by a model dependency payload so a downstream model can extend an upstream one. ## Notes for the reviewer - `tests/Opc.Ua.SourceGeneration.Tests` is left at its `master` content here. Its new vendor-subtype test uses the Robotics NodeSet as its fixture, so it can only land with the Robotics change at the top of the stack (5/5). - `PumpInstanceNodeIdRegressionTests` gains six `InputArguments`/`OutputArguments` entries for argument-less standard condition methods. Typed method signatures materialise those properties from the `ConditionType` declaration even where the method takes none. That is a real deviation, pinned rather than hidden so it stays visible; it is worth a follow-up. ## Validation `dotnet build UA.slnx` — 0 errors. | Suite | Result | | --- | --- | | Opc.Ua.SourceGeneration.Core.Tests | 3739 / 0 | | Opc.Ua.SourceGeneration.Tests | 81 / 0 | | Opc.Ua.Di.Tests | 304 / 0 | | Opc.Ua.History.Tests | 506 / 0 | | Opc.Ua.Gds.Tests | 1095 / 0 | History and GDS are listed because they are what the method-declaration fix above is load-bearing for: without it they fail with 10 and 370 errors respectively. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-08-01 07:55:57 +02:00
Marc Schier	8a10139b8c	Fix OPC UA address-space compliance in the device integration stack (#4117) ... # Description A live OPC UA compliance audit of `samples/PumpDeviceIntegrationServer` against the **OPC 40223 Pumps** and **OPC 10000-100 (DI)** companion specifications found **13 address-space violations** plus a serialization bug in the MCP tool. Every fix lands at its **root cause in the shared stack** rather than as a workaround in the sample, so all servers benefit. The sample now exposes **N fully simulated pumps** (`--pumps N`) and advertises the DI conformance facets it actually satisfies. > This branch has been merged with `master` including **#4116** ("Add simulation to PumpDeviceIntegrationServer and Event modelling"), which independently reworked the same pump simulation and the same fluent alarm builders. See [Relationship to #4116](#relationship-to-4116) below for how the two were reconciled. ## Findings and fixes | # | Finding | Root cause | Fix | |---|---------|-----------|-----| | 1 | `OverTempAlarm` created by the fluent API did not exist in the address space | `AttachAlarm` attached the alarm with `AddChild` but never indexed it into the node manager | Register the node; the same gap existed in `AddObject`, `CreateInstance` and the state-machine creators | | 2 | Alarms were unsubscribable — `EventNotifier = 0` everywhere, no `HasNotifier` from `Server` | No event-source wiring at all | Set `HasCondition`, initialise `SourceNode`/`SourceName`/`ConditionName`/`InputNode`, promote `EventNotifier` up the ancestor chain, publish `HasNotifier` from the `Server` object | | 3 | Every instance NodeId was minted in the **DI standard namespace** (`ns=4;s=5001_Pump #1`) | `DiNodeManager.New` inherited the parent's namespace index | New `DiNodeManager.InstanceNamespaceIndex`; instances move to the server's application namespace | | 4 | `Server/Namespaces` described only 2 of 6 namespaces | The NodeSet2 importer drops nodes parented to namespace 0, so companion-spec metadata objects never reached the address space — and no route covered the server's own namespace | New `NamespaceMetadataPublisher` walks the `NamespaceArray` and fills version/publication date from `ModelDependencyAttribute` | | 5 | Vendor BrowseNames (`Diagnostics`, `LastError`, …) sat in the DI namespace | Builders defaulted to the parent's namespace | Default to the server namespace; **spec-defined BrowseNames are unchanged** | | 6 | `AccessLevel="5"` from the Pumps NodeSet was reported as `1` | The ModelDesign `AccessLevel` enum is not `[Flags]` and cannot represent `CurrentRead \| HistoryRead` | Carry the verbatim NodeSet2 bitmask and emit named `AccessLevels` constants | | 7 | `Historizing = true` while `HistoryRead` returned `BadHistoryOperationUnsupported` | Nothing reconciled the declared history surface with the wired historians | Startup clears the advertisement (including masking the attribute read callbacks) when no provider resolves; the sample wires a real historian | | 8 | Mandatory `ProductInstanceUri` unset | Sample omission | Populated on every pump | | 9 | Mandatory `TrueState`/`FalseState` empty on every supervision boolean | Absent in the official NodeSet; a server must still populate them | Populated on every pump | | 10 | Machinery `Machines` folder empty (OPC 40001-1 §9.2 says **shall**) | `TryAddToMachinesFolder` used `AddChild`, which *re-parents* the device away from `DeviceSet` | Add the `Organizes` reference instead | | 11 | BrowseName contained a space and `#` | Sample naming | `Pump_1` with the readable label in `DisplayName` | | 12 | `Maintenance` group materialised but empty | Sample omission | Populated | | 13 | Uninitialised values reported `Good` | No initial-value status | `BadWaitingForInitialData` until the first simulation tick | | — | `ServerProfileArray` advertised only `StandardUA2017` | `DiNodeManager` did not implement the existing `IConformanceContributor` | Contribute DI 1.05 server facets computed at runtime from what is actually wired, merged with `StandardUA2017` | | — | MCP tool serialised boolean `false` and numeric `0` as JSON `null` | `Variant.Null` is `default`, so `Variant.Equals` adopts the non-null type and compares against the default value | Dispatch on `BuiltInType` and use typed `TryGetValue` accessors instead of the prohibited `AsBoxedObject` | ## Relationship to #4116 #4116 landed on `master` while this branch was in flight and targeted overlapping ground. The merge is a **union of both intents**, not a fast-forward: - **`AlarmBuilderExtensions`** — kept both sides. #4116 contributes the parent-must-be-an-Object guard, `SetEnableState`, and the `HasEventSource` references; this branch contributes the `HasCondition` reference, condition source initialisation, **registration of the alarm with the node manager so it is browsable**, notifier promotion up the whole ancestor chain, and root-notifier registration. - **`SupervisionBuilderExtensions`** — took #4116's `SetAlarmActive`. It is more spec-correct than this branch's version: it honours `EnabledState`, resets Acked/Confirmed on activation, and computes `Retain` per OPC 10000-9 so an unacknowledged Condition stays retained after going inactive. This branch's null guard was kept so a malformed alarm cannot dereference a missing `EnabledState`. - **`PumpNodeManager.Configure.cs`** — took #4116's **push-based** simulation (`RegisterPumpSimulation`, `CreatePumpSimulation`, `IValueUpdater<T>`, `Initialize`/`Advance`/`Publish`, deterministic per-pump phase offsets) as the structure, then folded in this branch's compliance work: configurable pump count, Maintenance group, `TrueState`/`FalseState`, historian wiring, per-pump mandatory Identification values, and `BadWaitingForInitialData` until the first tick. - **README** — union, keeping #4116's address-space validation workflow section. Two test adjustments were needed, both because the merge legitimately changed behaviour the tests pinned: - `PumpHostedReferenceTests` now resolves pump NodeIds from the browse result instead of hard-coding the DI namespace, and waits for the first published value. `PumpCreatedAfterStartupJoinsTheLiveSimulationAsync` was **added** to restore #4116's coverage that a pump created after startup through `ConfigureDevicesFor` joins the live simulation. - `FluentAlarmRegistrationIntegrationTests` no longer asserts that `Retain` clears when an alarm goes inactive — that pinned this branch's weaker semantics, and #4116's Part 9 behaviour is correct. ## Breaking change Runtime-created DI device instance NodeIds move from the companion-spec namespace to the server's application namespace, e.g. `ns=<DI>;s=5001_Pump #1` becomes `ns=<application>;s=5001_Pump #1`. This invalidates NodeIds cached by clients from earlier 2.0 previews. It is **not** a model change — standard DI/Machinery/Pumps type NodeIds and spec-defined BrowseNames are untouched. Device integration hosting is new in 2.0, so there is nothing to migrate from 1.5.378 and this is deliberately *not* in `docs/MigrationGuide.md`. Anyone tracking 2.0 previews should rediscover devices by browsing `DeviceSet` rather than persisting NodeIds, and resolve namespace indexes from `NamespaceArray` per connection. ## Verification Compliance is pinned by regression tests that assert **through the server's service surface** (`BrowseAsync` / `ReadAsync` / `HistoryReadAsync`), not by inspecting `NodeState` objects in memory. That distinction mattered: three defects survived in-memory assertions and were only caught through the read path — most subtly, `Historizing` was cleared while `AccessLevel` still advertised `HistoryRead`, because attribute read callbacks re-applied the NodeSet bits. Post-merge on net10.0: | Suite | Result | |---|---| | `Opc.Ua.Di.Tests` | 326 passed | | `Opc.Ua.Server.Tests` | 3846 passed, 5 skipped | | `Opc.Ua.Tools.Tests` | 288 passed | | `Opc.Ua.History.Tests` | 506 passed, 27 skipped | | `Opc.Ua.SourceGeneration.Core.Tests` | 3722 passed, 8 skipped | The full solution was also built and tested on **net48** before the merge. Review feedback from `copilot-pull-request-reviewer` has been addressed in commit `badad31dc`: the shared `ServerObjectState` lock was removed (the forward `HasNotifier` edge is published through the owning node manager instead), MCP array serialization now preserves element types, and the pump option guards report the specific rejected option. The only failure is `ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a pre-existing certificate-store issue that **passes when run in isolation** and is unrelated to this change. ## Documentation Updated `docs/DeviceIntegration.md`, `docs/MigrationGuide.md`, `docs/HistoricalAccess.md`, `docs/DeveloperGuide.md`, `docs/McpServer.md`, `docs/README.md`, `samples/PumpDeviceIntegrationServer/README.md` and `tools/Opc.Ua.Mcp/README.md`, and consolidated the four node-manager documents into a single [`docs/NodeManagers.md`](docs/NodeManagers.md) — overview, built-in managers (including `MasterNodeManager`), core vs custom, registration, server address-space metadata, and source generation — with a TOC and all inbound links repointed. ## Related Issues No tracking issue exists for this work yet — it originated from an ad-hoc compliance audit of the pump sample. Given the size of the change and the runtime-breaking NodeId move, please open (or link) a tracking issue so the design is recorded as an ADR before merging. - Fixes # ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d50c3fc-58b1-4c99-85a4-1e32c88666d9	2026-07-30 16:04:18 +02:00
Marc Schier	cfd15f08e1	Assign instance NodeIds in generated CreateOrReplace<Child> helpers (#4104) ... # Description The model source generator emits two families of instance helpers with inconsistent NodeId behaviour: * `ISystemContext.CreateInstanceOf<Type>(parent, browseName)` rebases the whole materialised subtree onto per-instance NodeIds. * `<Owner>.CreateOrReplace<Child>(context, replacement)` — the plumbing behind `NodeState.CreateChild` / `ReplaceChild` — did not. The child it produced was either bare (null NodeId, no browse name) or kept the type-level NodeId of the replacement. Any node manager mixing the two had to hand-roll a recursive re-stamp before registration, or sibling instances collided on NodeIds — a failure that only surfaces at registration time. `samples/PumpDeviceIntegrationServer` did exactly this. ## What changed **Generator** (`tools/Opc.Ua.SourceGeneration.Core`) * `CreateOrReplace<Child>` gains `bool assignInstanceNodeIds = true`. When enabled and the context supplies an `ISystemContext.NodeIdFactory`, a child whose NodeId is null or still equal to its declaration NodeId — and its descendants — are rebased through `NodeInstanceExtensions.AssignInstanceNodeId` / `AssignInstanceChildNodeIds`, with the owner as reference root. **A NodeId the caller already assigned is never overwritten.** * The helper also stamps the new child's `SymbolicName` / `BrowseName` / `DisplayName`. Without an identity, a path-based `INodeIdFactory` derives `{parent}_` for *every* child of a parent, so the assignment would have introduced collisions rather than removed them. `BrowseName` stamping is guarded on `context.NamespaceUris != null` — event-reporting paths pass a context without a namespace table. * All generated `Create*` factory call sites pass `assignInstanceNodeIds: false`: they build declaration subtrees whose NodeIds must stay at their type-level values, and the enclosing `CreateInstanceOf<Type>` factory rebases the finished subtree in a single pass. * The `CreateInstanceOf<Type>` rebase guard drops the `parent != null` requirement — supplying a `browseName` is what marks a dynamically materialised instance. Calling it without one (as the generated `NodeStateActivator`s do, and when replacing a well-known singleton) still keeps the declaration NodeIds. * `replacement` is now emitted as `BaseInstanceState?` on `CreateOrReplace<Child>` and on the generated `FindChild` override, matching the documented contract and the base `NodeState.FindChild` signature. **Hand-written twins** — `MethodState.CreateOrReplace{Input,Output}Arguments` and `BaseDataVariableState.CreateOrReplaceEnumStrings` gain the same parameter and behaviour, sharing a new internal `NodeInstanceExtensions.AssignNewChildInstanceNodeIds`. This is also required so the generated `public new` overrides keep hiding a matching base signature (otherwise `CS0109` with `TreatWarningsAsErrors`). **Sample** — `PumpDeviceIntegrationServer` now passes the real `DeviceSet` parent to the factory and drops `AssignChildNodeIds` plus the manual root stamp. **Docs** — new *"Materialising instances at runtime — NodeId assignment"* section in `SourceGeneratedNodeManagers.md` and a refreshed `DeviceIntegration.md`. ## Design note: why not the guard proposed in the issue Gating on *"the owner already has an instance NodeId"* cannot be implemented as `owner.NodeId != <ownerTypeNodeId>`. The generated child factories call `<Owner>.CreateOrReplace<Child>` on **declaration** instances — `CreatePumpType_Operational` runs on a `FunctionalGroupState` whose NodeId is `ObjectIds.PumpType_Operational`, not `ObjectTypeIds.FunctionalGroupType` — so the comparison would be true while the *type template* is being materialised (`forInstance: false`) and would rewrite the address space's declaration NodeIds. The explicit opt-out separates the two callers unambiguously. ## Compatibility Adding an optional parameter is source-compatible. Behaviour changes for callers of `NodeState.CreateChild` / `ReplaceChild` when the context carries a `NodeIdFactory`: the new child now carries a browse name and a per-instance NodeId instead of nothing. ## Related Issues - Fixes #4099 > [!NOTE] > The issue asks to delete `AssignChildNodeIds` from `samples/MinimalRobotServer`. That sample does not exist on `master` — it only lives on the still-open #4096 branch and carries no such helper. The equivalent boilerplate on `master` is `PumpDeviceIntegrationServer/PumpNodeManager`, which this PR cleans up. Details in [this comment](https://github.com/OPCFoundation/UA-.NETStandard/issues/4099#issuecomment-5102275507). > [!NOTE] > One commit is an unrelated one-line-scope build-hygiene fix: two `CA2000` warnings in `ConsoleReferenceClient` where subscription ownership transfers to the session. Happy to split it out if preferred. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. ### Tests added **Generator emission** (`NodeManagerGeneratorTests`) — `CreateOrReplaceChildAssignsInstanceNodeIdsByDefault`, `TypeFactoriesOptOutOfCreateOrReplaceNodeIdAssignment`, `CreateInstanceOfFactoriesRebaseWithoutAnExplicitParent`. **Helper behaviour** (`NodeInstanceExtensionsTests`) — `CreateOrReplaceArgumentsAssignsInstanceNodeIds`, `CreateOrReplaceArgumentsKeepsCallerAssignedNodeId`, `CreateOrReplaceArgumentsHonoursTheAssignmentOptOut`, `CreateOrReplaceEnumStringsAssignsInstanceNodeIds`. **Pump DI server regression** (`PumpInstanceNodeIdRegressionTests`, added on review feedback): | Test | Asserts | | --- | --- | | `ConfiguredPumpExposesTheFullModelSurface` | The exact set of **77 model identifiers** a configured pump materialises. | | `EveryPumpInstanceExposesTheSameGeneratedModelSurface` | A second pump exposes the identical generated surface, minus what the fluent `Configure` pass adds. | | `MaterialisedPumpNodeIdsComeFromTheNodeIdFactory` | Every NodeId is `{parentIdentifier}_{symbolicName}` in the parent's namespace — minted by `PumpNodeManager.New`. | | `MaterialisedPumpNodeIdsAreUniqueAcrossInstances` | No NodeId is null or shared between two instances of the type. | | `GeneratedFactoryAssignsInstanceNodeIdsBeforeRegistration` | The factory output already carries per-instance NodeIds before `AddPredefinedNodeAsync` applies its defensive rebase. | | `AlarmSubtreeKeepsStandardDeclarationNodeIds` | Pins the alarm subtree's standard-namespace ids — a pre-existing gap outside the generated helpers. | The 77-identifier baseline was captured by running the same subtree walk against the pre-change commit and diffing it against the post-change run: **identical** — same node set, same NodeIds. ### Local test runs | Suite | net10.0 | net48 | net472 | | --- | --- | --- | --- | | `Opc.Ua.SourceGeneration.Core.Tests` | 3722 passed | 3721 passed | — | | `Opc.Ua.SourceGeneration.Tests` | 81 passed | 81 passed | — | | `Opc.Ua.SourceGeneration.Stack.Tests` | 90 passed | — | — | | `Opc.Ua.Types.Tests` | 8083 passed | 8076 passed | — | | `Opc.Ua.Di.Tests` | 303 passed | 303 passed | 277 passed | | `Opc.Ua.Core.Tests` | 4134 passed | — | — | | `Opc.Ua.Gds.Tests` | 1095 passed | — | — | | `Opc.Ua.History.Tests` | 506 passed | — | — | | `Opc.Ua.Server.Tests` | 3626 passed, 1 failed | 3574 passed, 1 failed | — | The single `Opc.Ua.Server.Tests` failure — `ServerFluentApiHostingTests.ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync` — is pre-existing and unrelated; it reproduces on a stashed clean `master` tree. > [!NOTE] > The source-generation test harness compiles generated model code against a hand-written `Opc.Ua` API stub (`tests/Opc.Ua.SourceGeneration.Core.Tests/CompilerUtils.cs`, shared by three test projects). Its 73 `CreateOrReplace<Child>` declarations were updated to the new signature — without that, every generated call site passing `assignInstanceNodeIds: false` failed to bind. `NodesetMethodArgumentTests` pinned the exact emitted call and was updated to the new one for the same reason as the `NodeStateGeneratorTests` expectations. > [!NOTE] > `master` has been merged into this branch. The only conflict was in `NodeStateGeneratorTests.cs`, where master rewrapped the expected call-site strings while this branch appended `assignInstanceNodeIds: false` to them — kept master's wrapping and re-applied the argument. Master's two new templates (`Create_MethodArguments`, `Assign_MethodArgumentValues`) call `CreateOrReplace{Input,Output}Arguments` while building a declaration subtree, so they now pass `assignInstanceNodeIds: false` to preserve their stated intent of leaving the property node's identity untouched. > [!TIP] > Changing a source generator leaves dependent projects' cached generated output stale. If a local build reports `CS1739: ... does not have a parameter named 'assignInstanceNodeIds'` or `CS0109` on a generated `CreateOrReplace*`, run `dotnet build <project> -t:Rebuild`. Clean CI builds are unaffected. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b019f3a2-f58d-4f7e-93db-c8a0dec656e2	2026-07-28 21:22:14 +02:00
Marc Schier	5b9bcedc62	Add OPC UA ISA-95 Common Model and Job Control support (#4089) ... # Description Adds first-class support for the OPC UA ISA-95 companion specifications: - OPC-10030 Common Model. - OPC-10031-4 Job Control V1. - OPC-10031-4 Job Control V2. The change introduces `Opc.Ua.ISA95`, `Opc.Ua.ISA95.Client`, and `Opc.Ua.ISA95.Server`, together with a NativeAOT-compatible minimal server sample and comprehensive tests. ## Highlights - Compiles the three canonical NodeSets and identifier mapping CSV files through the source generator. - Adds NodeSet identifier mapping validation, typed method arguments/results, companion event records, and generated method argument nodes. - Provides typed Common Model builders with normative relationships and an injectable `GeoSpatialLocationType` provider. - Implements lossless V1/V2 Job Control providers over a shared version-neutral store, including state paths, status events, comments, response queries, catalog changes, and cohesive DI registration. - Adds subtype-aware typed clients, standalone encodeable registration, `IIsa95ClientFactory`, and V2 status-event streaming. - Adds the `MinimalIsa95Server` sample, package documentation, and an ISA-95 guide with conformance and HA guidance. - Tracks typed RSL/GPOS integration as follow-up issue #4095. ## Validation - `Opc.Ua.ISA95.Tests`: 137/137 passed on `net10.0` and `net48`. - Source-generator suites: 81/81 model, 90/90 stack, 3718 passed / 8 skipped core. - `EventRecordFieldReaders` feedback tests: 6/6 on `net10.0` and `net48`. - `EventRecordFieldReaders` changed-method patch coverage: 100% line coverage (36/36) and 100% branch coverage (22/22). - `Opc.Ua.ISA95`, `.Client`, and `.Server` build across all configured target frameworks without new diagnostics. - Cobertura line coverage: 88.08% model, 90.04% client, 91.19% server. - NativeAOT sample publishes and passes a live typed V2 Store/status-event validation. - The ISA-95 NativeAOT smoke test passes in the source-generated native test executable. - OPC UA KB NodeSet validation reports zero errors for Common, V1, and V2. The broader NativeAOT executable still reports unrelated existing GDS and complex-type test failures; the ISA-95-specific native test passes. ## Node state generator fixes Generating method arguments inline for imported nodesets regressed every model that already emits `InputArguments` / `OutputArguments` child node factories: - The inline block ran before the child factories. `MethodState.CreateOrReplaceInputArguments` keeps the property that was assigned first, so the child carrying the well-known NodeId was dropped and a NodeId-less placeholder remained in the address space. This broke browsing (reference descriptions without a target) and made method arguments unreachable at their well-known NodeIds. - The `MethodDeclarationId` fallback keyed on an autogenerated display name redirected every declared method of the standard address space to its own NodeId, which made method calls fail. The inline block is now emitted after the child factories and reduced to a value assignment when a child factory exists. The value is only re-emitted when an argument references a namespace other than OPC UA, because the value of an imported property is the verbatim XML of the source nodeset whose namespace indices do not survive the import. The `MethodDeclarationId` fallback is restricted to declarations without a resolvable NodeId. The generated OPC UA address space is byte-identical to `master` again. ## Related Issues - Fixes #4088 - Follow-up: #4095	2026-07-28 12:01:57 +02:00
Marc Schier	38c7356bc7	Add generic xRegistry libraries (abstract registry base model) (#4097) ... # Description xRegistry is the *abstract registry base model*: a registry-agnostic, content-addressed resource registry. It knows nothing about what a resource contains — a concrete registry supplies its own companion namespace and a fingerprinting strategy and reuses everything else. The PubSub Schema Registry in #4007 is the first specialization (its resources are schema documents), and #4093 (WoT Binding) carries a byte-identical copy of the same base NodeSet, so a shared canonical home is worth having. ## Packages | Package | Depends on | Contains | | --- | --- | --- | | `OPCFoundation.NetStandard.Opc.Ua.XRegistry` | `Opc.Ua.Core` | Abstract base companion model (compiled by the model source generator), `XRegistryWellKnown`, `IResourceContentIdProvider` | | `OPCFoundation.NetStandard.Opc.Ua.XRegistry.Client` | + `Opc.Ua.Client` | `XRegistryClient` (abstract) / `GenericXRegistryClient` (sealed), built on the generated ObjectType proxies | | `OPCFoundation.NetStandard.Opc.Ua.XRegistry.Server` | + `Opc.Ua.Server` | Fast-path / registration / federation node managers, `XRegistryServerOptions`, `IXRegistryResourceStore` | `Opc.Ua.XRegistry` deliberately depends on neither SDK, so a codec or a shared contracts assembly can reference the identity abstraction without pulling in the client or the server. ## The model is compiled, not loaded The companion model is compiled into the assemblies by the OPC UA model source generator. `Opc.Ua.XRegistry.NodeSet2.xml` is an `AdditionalFiles` generator input only — there is no `EmbeddedResource`, no runtime NodeSet parsing and no hand-written NodeId tables for anything the model declares. All three node managers load it the same way: ```csharp protected override NodeStateCollection LoadPredefinedNodes(ISystemContext context) { return new NodeStateCollection().AddOpcUaXRegistry(context); } ``` `XRegistryWellKnown` is reduced to the namespace URI plus the identifiers of the *instances* a registry materializes at runtime, which live above the model's own 63000-63999 range so they can never collide with it. ## What it does - **Content-derived identity** — `IResourceContentIdProvider` maps a document + format to a fingerprint and names the (canonicalization, hash) algorithm. Identity is derived from the bytes, not assigned by the server, so it is stable across registries — which is what makes de-duplication and federation work. - **Opaque-NodeId fast path** — a registered resource is reachable at an Opaque `NodeId` whose Identifier is the raw content-id bytes, so a consumer that received the id on the wire resolves the document in a single `Read`: no Browse, no fingerprint recomputation. The client reads it through the `ReadBytesAsync` session extension, so a document larger than `MaxByteStringLength` is fetched with range-based reads rather than failing. Optionally pre-publishes a seed resource so a fresh server can resolve one resource before any registration. - **The model's own lifecycle** — the registry root is a `RegistryType` instance serving `CreateGroup` / `GetOrCreateGroup`; each group serves `CreateResource` / `GetOrCreateResource`; and because `ResourceType : FileType`, the document *is* the file — content is streamed through the inherited `Open` / `Read` / `Write` / `Close`. `Close` fingerprints the accumulated bytes and publishes the Opaque fast-path node at runtime. Identical bytes reuse the existing node, whose lifetime is reference counted so it survives until the last resource resolving to those bytes is gone and is retired when a resource is re-written with different content. - **Optimistic concurrency** — `Delete(ExpectedEpoch)` on both resources and groups, and the same epoch check on label mutations, so a caller working from a stale read is rejected rather than clobbering a concurrent change. - **Labels** — `RegistryType`, `GroupType` and `ResourceType` each expose an `AttributesType` Object with `AddAttribute` / `RemoveAttribute`; labels are published as addressable `String` Properties. - **Federation** — a resource hosted by another registry is published as a real `ResourceType` instance whose `ExternalReference` (`ExpandedNodeId` + `ServerIndex` into the local `ServerArray`) and `ResourceUrl` carry the link and whose `Xid` carries the content-id. Because it is an ordinary `ResourceType`, a generic client drives a federated resource through exactly the same proxy as a local one. ## Extensibility A domain registry subtypes the base model — the PubSub Schema Registry declares `SchemaFileType : ResourceType` — and the generator mirrors that hierarchy across assemblies, so the proxy chain follows automatically (`SchemaFileTypeClient : ResourceTypeClient : FileTypeClient`). ```text abstract XRegistryClient ├── sealed GenericXRegistryClient // any registry namespace ├── SchemaRegistryClient (domain) └── WotRegistryClient (domain) ``` * A **domain client** derives from `XRegistryClient` and inherits the whole lifecycle. Helpers written as extension methods over a base proxy (such as `WriteDocumentAsync` on `ResourceTypeClient`) are callable on the domain proxy with no inheritance in the client layer. * A **generic client** still drives a domain registry, because a domain instance *is* an instance of the base type. This is covered by a test that defines a domain subclass over the base. ## State Document bytes live behind an injectable `IXRegistryResourceStore`. Because a resource is a `ResourceType`, which *is* a `FileType`, the store mirrors the file access model: reads and writes are **offset and length based**, so a document never has to be materialized as a whole and a ranged read touches only the bytes it needs. The document is transferred with the inherited `FileType` Methods, whose mode bits are honoured: a write that does not set `EraseExisting` starts from the stored bytes and replaces only the range it writes rather than truncating the rest. A handle is valid only on the resource and the session that opened it, and a session's handles are released when it closes. Two implementations ship: `InMemoryResourceStore` (the default, in-process) and `FileSystemResourceStore`, built on the `IFileSystem` abstraction so documents outlive the process and a shared volume can back a cluster. `XRegistryResourceStoreContractTests` is a reusable fixture that validates any implementation against the contract. ## Transport security Registry **writes always require a `SignAndEncrypt` secure channel** — a document and its content-derived identity are integrity-critical, so every mutation is rejected with `BadSecurityModeInsufficient` on a channel that is only signed or unprotected. This is not configurable. Reads are allowed on any secure channel by default; `RequireEncryptionForReads` extends the requirement to them when the documents themselves are confidential. An in-process call carries no channel and is always allowed. ## Hardening The lifecycle Methods are remotely callable, so every unbounded dimension is bounded via `XRegistryServerOptions`; exceeding a bound fails the call rather than the server: | Option | Default | Enforced on | Status code | | --- | --- | --- | --- | | `MaxConcurrentUploads` | 64 | `CreateResource`, `Open` | `BadTooManyOperations` | | `MaxResourceBytes` | 16 MiB | `Write` | `BadRequestTooLarge` | | `MaxRegisteredResources` | 4096 | `CreateResource` | `BadTooManyOperations` | ## A source generator bug fixed along the way Building on the compiled model surfaced a defect in shared generator infrastructure. `NodeSetToModelDesign.CreateDecoder` decoded values with an empty `EncodeableFactory`, so a NodeSet2's method `InputArguments` / `OutputArguments` — encoded as a list of `Argument` ExtensionObjects — never decoded, while `HasArguments` was still set. **Every** NodeSet2-sourced model in the stack was silently generating argument-less `MethodState`s and ObjectType proxies. ModelDesign-sourced models (the core stack) were unaffected, which is why this went unnoticed. Fixed in `b87726af` with a regression test that fails without it. Two defects in the xRegistry NodeSet itself were fixed at the same time: `BrowseName="1:InputArguments"` → ns-0 `InputArguments` (published NodeSets never namespace-qualify these), and the `CreateResource` / `GetOrCreateResource` **output** `VersionId` renamed to `AssignedVersionId`, which collided with the input of the same name and produced a duplicate C# parameter. ## Other fixes applied while extracting - `XRegistryRegistrationNodeManager` uses `System.Threading.Lock` instead of an `object`-typed sync root, per the repository locking rules. - Dropped the `InternalsVisibleTo` entries granting internals to `Opc.Ua.PubSub.Server.Tests`. That project only consumes public `XRegistryWellKnown` API. - The `UA.slnx` edit in #4007 accidentally **replaced** the `tests/Opc.Ua.Redundancy.Kubernetes.Tests` entry instead of adding a line, silently dropping that project from the solution. The entry is preserved here and the change is purely additive. ## Tests `tests/Opc.Ua.XRegistry.Tests` (168) covers the group and resource lifecycles, the `FileType` transfer, delete and epoch conflicts, labels, federation, both resource stores, the fast path, transport security and the extensibility contract — positive and negative paths. Passing on `net10.0` and `net48`. Patch line coverage is **98.5%**. Also adds `tests/Directory.Build.props` / `.targets`: the .NET 10 SDK prunes `Microsoft.Extensions.Logging.Abstractions` out of the test output, and coverlet's Mono.Cecil resolver only searches the output directory, so it silently fails to instrument every module that references `ILogger` and reports 0% coverage for them. These force the package back into the restore graph and copy its implementation next to the test binary. ## Documentation New [`docs/XRegistry.md`](docs/XRegistry.md), linked from the documentation index.	2026-07-28 08:45:03 +02:00
Marc Schier	66be6731f4	Source generator: support compiled companion models and safe NodeState writes (#4063) ... ## Summary Combines the compiled companion-model support from #4062 with the NodeState emission fixes already in this PR. The model source generator can now consume compiled companion `NodeSet2` models more reliably, while generated factories consistently target base `NodeState` members even when a generated subclass shadows them. ## Changes ### Compiled companion NodeSets - Add `OpcUaStateTypeIndex` and wire it through `ModelSourceGenerator` / `ModelCompilation` so standard (`ns=0`) node-state and method-state class names are resolved from referenced `Opc.Ua.Core(.Types)` metadata. - Add the model-generation-scoped `StandardMethodStateFallback`, which degrades an unresolved standard typed `*MethodState` reference to `global::Opc.Ua.MethodState` only when the type is neither available in the compilation nor declared by the current generation pass. - Keep declaration names outside the reference-only fallback policy, propagate fallback state through the logical execution context, reuse the immutable availability index, and use named incremental-generator inputs instead of deep `Left` / `Right` chains. - Escape generated fluent-builder XML documentation where browse names can contain XML-sensitive characters. ### Safe NodeState factory emission - Declare one base-typed `global::Opc.Ua.NodeState nodeState = state;` local per generated factory. - Use `nodeState` for base-member reads and writes to `NodeId`, `SymbolicName`, `BrowseName`, `DisplayName`, `Description`, `WriteMask`, `UserWriteMask`, `AccessRestrictions`, `ReleaseStatus`, `Categories`, `Specification` and `IsPartOfTypeHierarchy`. - Base-qualify the optional-child `previousNodeId` read, NodeId comparison, and existing-child NodeId update. - `global::`-qualify emitted `NodeId` / `ExpandedNodeId` namespace prefixes to avoid namespace collisions. ### Supporting updates - Expose `Opc.Ua.Types` internals to `Opc.Ua.Core.Encoders.Tests`. - Update generator assertions and add focused fallback regression coverage. ## Why Compiled companion models can instantiate standard types whose curated Core implementation does not expose the conventionally named typed method-state class. The scoped fallback lets those models generate without weakening the Stack generator or masking classes emitted by the current model. Separately, generated subclasses can shadow base `NodeState` properties with strongly typed `new` properties. Routing base attributes through a `NodeState`-typed local guarantees initialization reaches the intended base member while keeping generated code smaller and clearer than repeated casts. ## Validation - Full `Opc.Ua.SourceGeneration.Core.Tests` on net10.0: **3718 passed / 8 skipped / 0 failed**. - Combined focused generator coverage on net48: **21 passed / 0 failed**. - Full `Opc.Ua.SourceGeneration.Tests`: **68 passed / 0 failed**. - Full `Opc.Ua.SourceGeneration.Stack.Tests`: **90 passed / 0 failed**. ## Context This PR supersedes #4062 and contains both source-generator change sets. The companion-model fallback remains the scoped interim workaround tracked by #4060; a future upstream name-resolution step can remove it. These changes also feed the experimental data-encodings work in #4007. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-21 20:48:44 +02:00