UA-.NETStandard

mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-26 04:19:53 +08:00

Author	SHA1	Message	Date
romanettandClaude Opus 5.5	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
romanettandClaude Opus 5.5	5572107ab8	Push Model compliance: UpdateCertificate issuer check, TrustList audit events, NoConfiguration state, DefaultHttpsGroup (#4684 ) # Description Fixes found by a manual run of the OPC 10000-12 Push Model for Global Certificate and TrustList Management test plan (183 test cases, TC0–TC182) against the Reference Server. After these changes no test case fails; the remaining non-pass verdicts are test-plan expectations that contradict Part 12/Part 4/Part 20 (documented in the run report). ### Server behaviour changes - UpdateCertificate requires the issuer in the TrustList (`ConfigurationNodeManager.PushValidation.cs`). The validator classifies an incomplete chain as suppressible; OPC 10000-4 Table 100 does not ("An error during the chain creation may not be suppressed") and §7.10.5 requires the issuers to already be in the group's TrustList. A certificate of an unknown CA is now rejected with `Bad_CertificateChainIncomplete`. Expiry / host-name errors stay ignored as §7.10.5 requires. - TrustList audit events reach the Server Object. `TrustListUpdateRequested`/`TrustListUpdated` were reported on the TrustList node (`EventNotifier=0`, not linked to the Server), so no client could receive them. `TrustList.SetAuditEventServer` routes them through `IAuditEventServer`; wired for the Push TrustLists and the GDS CertificateGroup TrustLists. New `AuditEvents` overloads keep the node-based ones. - Duplicate AddCertificate no longer makes the following ApplyChanges fail with a bare `Bad` (store refused the second copy); the commit is idempotent and its rollback keeps the pre-existing certificate. - CloseAndUpdate validates CRLs (incl. CRL-only updates): every CRL must be signed by a CA of the new TrustList (§7.8.2.5). Previously an unknown/forged CRL passed CloseAndUpdate and only failed ApplyChanges. - X.509 v1 certificates are rejected by CloseAndUpdate/AddCertificate/UpdateCertificate (OPC 10000-6 §6.2.2; new `X509Utils.IsX509Version3`). - Application setup state (Annex G.2): new `StandardServer.StartupServerState` (virtual); the DI host returns `NoConfiguration` when `ServerConfigurationOptions.InApplicationSetup == true`. `IsRunning` and `NodeManagerLifecycle` treat `NoConfiguration` as operational. The Reference Server `--provision` mode starts in `NoConfiguration` and exposes `InApplicationSetup=TRUE`. - DefaultHttpsGroup: the group is only created when Https stores are configured — the Reference Server config had none, so its `opc.https`/`opc.wss` endpoints used the RSA application certificate with an empty TLS trust list and the Https certificate could not be pushed. The sample config now declares `HttpsIssuerCertificates`/`TrustedHttpsCertificates` and an `HttpsCertificateType` certificate (new `CertificateTypeString` value `Https`). An Https certificate is assigned to DefaultHttpsGroup only (no longer also listed in DefaultApplicationGroup). Fixed `AddSecurityConfigurationHttpsStore`, which built the trusted Https store path from the store type. - Minor: AddCertificate logs a warning when it adds a CA without a CRL while `RejectUnknownRevocationStatus` is enabled; UpdateCertificate refused for an insufficient SecureChannel raises `CertificateUpdateRequestedAuditEvent` with Status=false. ### Kept on purpose - PEM certificates are accepted (converted to DER). - `Bad_NotWritable` when opening an already open TrustList (OPC 10000-20 §4.2.2) and for AddCertificate while a read handle is open (§7.8.2.6). - No validation rules for DefaultUserTokenGroup / DefaultHttpsGroup TrustLists (Part 12 defines them only for ApplicationCertificateType groups). ### Notes for reviewers - Existing tests in `ConfigurationNodeManagerPushTests` and `PushCertificateRequestRegressionTests` signed pushed certificates with CAs that were not in the TrustList; they now trust their CA first (behaviour change above). - Adding the Https certificate to `Quickstarts.ReferenceServer.Config.xml` gives the Reference Server's opc.https/opc.wss endpoints their own TLS certificate. ### Tests - `tests/Opc.Ua.Server.Tests/Configuration/PushComplianceRegressionTests.cs` — duplicate add/rollback, PEM, v1/v3, CRL issuer/forged/CRL-only, audit routing. - `tests/Opc.Ua.Server.Tests/ApplicationSetupServerStateTests.cs` — NoConfiguration startup state. - `tests/Opc.Ua.Gds.Tests/PushComplianceTests.cs` — end-to-end over a secure channel: TrustList transaction lifecycle, open-handle rules, masked reads, duplicate/PEM add, unknown CRL issuer, unknown certificate issuer, cross-Session CSR key, nonce rules, RemoveCertificate chain/CRL handling. Mutation-checked: reverting the issuer, duplicate-add and CRL fixes makes the corresponding tests fail. - Additional cases in `PushCertificateRequestRegressionTests`, `CertificateIdentifierTests`, `ApplicationConfigurationBuilderTests`. Local runs (net10.0): Opc.Ua.Gds.Tests 1257 passed / 0 failed / 56 skipped; Opc.Ua.Server.Tests push/TrustList/audit/lifecycle subset 1209 passed; Core and Configuration test classes passed. net48 build of the affected test projects succeeds; `dotnet format whitespace` clean. ## Related Issues - None (findings from a manual Push Model compliance run). ## 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. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-06 18:48:02 +00:00
romanettandClaude Opus 5.5	7212607f16	OPC UA Part 3 compliance: permissions, Write semantics, NodeManagement, NodeSet DataTypes, locales (#4675 ) # Description Fixes the High and Medium findings of an OPC UA Part 3 (OPC 10000-3, Address Space Model) compliance audit. Each finding was checked against the full spec text (Parts 3, 4, 5, 6, 14 and 18) before it was fixed. Every fix comes with regression tests that fail without it. ### Role permissions (Part 3 §8.55, Part 4 §5.12.2.2) - The type-hierarchy exemption in `MasterNodeManager.ValidateRolePermissions` / `ValidateAccessRestrictions` now covers only Browse, Read and ReadRolePermissions. Type nodes stay visible to generic clients, which was the intent of #3705; every other permission bit is now enforced on them. - `ReceiveEvents` is enforced on the EventType node. The standard NodeSet grants it on `AuditEventType` and its subtypes only to SecurityAdmin, so only SecurityAdmin sessions now receive audit events. - Call checks the `Call` permission and AccessRestrictions on the Object as well as on the Method. - When a Call's `methodId` is the Method declaration of the ObjectType, the Object's Method with the same BrowseName is permission-checked and invoked. The declaration is used only if the Object has no such Method. This applies to `AsyncCustomNodeManager`, `CustomNodeManager2` and the sample node manager. ### Write service (Part 3 §5.6.2, §8.40, §8.57; Part 4 §5.11.4) - The synchronous `OnWriteValue` path (fluent `OnWrite`) checks DataType and ValueRank first (`Bad_TypeMismatch`). After an IndexRange write it merges the slice into the cached value instead of replacing it. - StatusWrite / TimestampWrite are enforced (`Bad_WriteNotSupported`). - Undefined enumeration values and invalid OptionSet writes return `Bad_OutOfRange`. OptionSet writes merge only the valid bits into the stored value. - New `ReportPropertyValueChanged` / `ReportSemanticChange` let server code raise SemanticChangeEvents for changes it makes itself. - Required follow-up (Part 14 §6.2.11.1): if an external target rejects a timestamp, the PubSub subscriber's TargetVariables writer retries without it. ### NodeManagement and Call arguments (Part 3 §5.5.2, §5.7.3, §6.4.2; Part 4 §5.8, §5.12.2) - AddNodes creates the Mandatory InstanceDeclarations of the TypeDefinition and rejects abstract or Interface types (`Bad_TypeDefinitionInvalid`). - AddReferences, DeleteReferences and DeleteNodes bump NodeVersion and raise GeneralModelChangeEvents. - Clients may omit trailing optional input arguments (`HasOptionalInputArgumentDescription`). ### NodeSet DataTypes (Part 3 §5.8.3, §8.48; Part 6 Annex F.12) - `UANodeSet.Import` completes structure definitions with the base type's fields (base fields first). It creates definitions for structures without fields and sets `DefaultEncodingId`. `StructureDefinition.FirstExplicitFieldIndex` marks the subtype's own fields. - `DataTypeState` no longer mutates the stored definition on read. ### Locales (Part 3 §5.2.5, §8.5; Part 4 §5.4) - Read, Browse and data change notifications return the translation that best matches the session's LocaleIds, or all translations for `mul`. The selection lives in the server dispatcher and is built on `ResourceManager`: the new `TranslateValue` calls the overridable `Translate`. - Locale ids are matched case-insensitively, and fallback translations carry their real locale. ### Client complex types (Part 6 §5.1.7) - The Reflection.Emit builder encodes only `IsOptional` subtyped fields as ExtensionObject. It also supports OptionSet subtypes: an unsupported type is skipped instead of aborting the whole load. - `TypeInfo.GetXmlName` uses the data type's XML name for dynamic structures and OptionSets (the XML body element was `<Structure>`). Both builders now produce identical XML. ## Behaviour changes All are documented in `docs/MigrationGuide.md`. - Audit events: only SecurityAdmin receives them. The CTT "Auditing Connections" unit needs a SecurityAdmin user (e.g. sysadmin); `AuditingOperationTests` was updated. - Write timestamps and status: clients writing a SourceTimestamp or a non-Good StatusCode get `Bad_WriteNotSupported` unless the Variable has StatusWrite/TimestampWrite. No variable in `src/` sets these bits. - AddNodes: it rejects abstract types (including `BaseVariableType` and `BaseEventType`), and new instances now come with child nodes. - Call: the Object needs the Call permission. The Object's own Method runs even when the call names the type declaration. ## Not included - Low-severity findings: missing validation on optional services and author-side NodeSet rules. - LocalizedText nested inside structures or Variant arrays, and in HistoryRead or Call outputs. ## Testing net10.0, Debug, one suite at a time on the merged branch. All suites below pass: \| Suite \| Passed \| \|---\|---\| \| Types \| 11430 \| \| Core.Encoders \| 2947 \| \| Core.Schema \| 179 \| \| Core.Security \| 558 \| \| Client \| 2761 \| \| Server \| 8210 \| \| InformationModel \| 1080 \| \| Features \| 145 \| \| History \| 536 \| \| Di \| 455 \| \| Machinery \| 167 \| \| Robotics \| 661 \| \| Robotics.Intent \| 22 \| \| Pumps \| 75 \| \| Scales \| 122 \| \| Positioning \| 69 \| \| ISA95 \| 144 \| \| Vision \| 484 \| \| PubSub \| 1490 \| \| PubSub.Adapter \| 151 \| \| PubSub.Server \| 177 \| \| Redundancy.Server \| 856 \| \| Lds \| 173 \| \| Configuration \| 241 \| \| WotCon \| 1496 \| - Client.ComplexTypes: one test wrote a SourceTimestamp to a variable without TimestampWrite. It is fixed in this PR, and the suite has not been re-run since. - Core: 6068 passed, 1 failure in `ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly`. That telemetry test passed when re-run alone, so it looks flaky. - Still running: Sessions, Subscriptions (Classic, Durable, standard) and Interop. Results will follow on this PR. - `UA.slnx` builds with 0 errors for net10.0 and net48, with no new analyzer warnings in changed files. ## Related Issues - Part 3 compliance audit (no tracking issue) ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-05 12:43:59 +00:00
romanettandClaude Opus 5.5	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
romanettandClaude Opus 5.5	4c1c3fd244	Aggregates: fix remaining CTT aggregate server defects and seed Bad data for the CTT (#4655 ) Fixes the remaining CTT Aggregate failures of the reference server that are server defects or missing test data, using the patched CTT 1.05.513 scripts (all known Mantis defects fixed) as the oracle. ## Server fixes (`src/Opc.Ua.Server`) - Real-valued bounds for TimeAverage/TimeAverage2/Total/Total2. `SlopedInterpolate` converted every interpolated bound to the raw data type, so on Int32 nodes these Double aggregates used rounded bounds (ramp 0..23.8: 11.9034 instead of 11.9). The conversion (Part 4 §7.7.3) is now only applied for aggregates that return the raw data type (Interpolative, Start/EndBound, DeltaBounds, Minimum2/Maximum2/Range2, ActualTime2); `AverageAggregateCalculator` keeps the real bound (Part 13 §3.1.8, Table 50). - Non-numeric values are interpolated stepped. Boolean, String and other non-numeric values cannot be interpolated with a sloped line (Part 13 §5.4.2.3, Table 50). The calculator now uses stepped interpolation for them whatever the Stepped property says: a String EndBound between "23" and "24" is "23" (was "23.8"), a Boolean bound between false and true is false, and Boolean DurationInState no longer counts the region before the first value as true. - TreatUncertainAsBad = false in Average and Count. Both used only strictly Good values for the result but counted Uncertain values as Good for the status. They now include Uncertain values when TreatUncertainAsBad is false (Part 13 §4.2.1.2), like StandardDeviation and NumberOfTransitions already did. - Interpolative honors TreatUncertainAsBad for a raw value at the interval start, as the calculator already did for the raw values around a bound. - NumberOfTransitions: a Bad result carries no aggregate bits, like Count. - Processed reads get the bounding values they need (`HistorianDispatcher`). The raw read only returned the raw value at or beyond each end of the request. When that value is Bad (or Uncertain with TreatUncertainAsBad), the interpolated bound needs the nearest Good value further out, so an interval starting on a Bad value returned `BadNoData` and the next interval lost its first region. The dispatcher now reads outwards to the nearest Good values (reusing the ReadAtTime bound search), only when the end value is not Good. ## Reference server test data and CTT template - Every scalar history node also gets 400 values at a fixed date (`2026-01-01T00:00:00Z`) with a block of five Bad values, laid out for the CTT's Bad-data search (`HAAggregateHelper.GetBadData`). StartOfArchive is the fixed date. The values relative to the server start are unchanged. - `samples/UAReferenceServer.ctt.xml`: all ten `.../Aggregates/StartOfBadData*` settings point at the block (`2026-01-01T00:25:01.234Z`), and `RequestedNodeId` is enabled with namespace 2 (needs the C34 fix). - `plans/ctt-issues.md` / `plans/ctt-testing.md`: updated configuration notes, the oracle differences, U4, and new U5 (TreatUncertainAsBad = false). Also documents a CTT defect: the Bad-data search's "reverse" read sends EndTime = 0001-01-01 (null), which a server reads forward. ## CTT results (p06 Aggregates, 1267 test cases) \| \| Error cases \| \| --- \| --- \| \| Before (patched scripts, origin/master) \| 253 \| \| This PR, patched scripts \| 125 \| \| This PR, patched scripts + proposed Int32 oracle corrections for TimeAverage/Total(2) and Range2 \| 51 \| The 125 include 78 TimeAverage/TimeAverage2/Total/Total2 and 4 Range2 test cases on Int32 nodes where the server now returns the Part 13 value but the patched oracle still truncates the bounds. The remaining 51 are spec questions: U4 (DeltaBounds, 9; TimeAverage2/Total2 with an Uncertain value after the start bound, 8) and U5 (TreatUncertainAsBad = false, 34). p07 Historical Access: no change (0 errors). ## Tests - New `AggregateBoundDataTypeTests` (real bounds on Int32, Interpolative still rounds, Boolean/String stepped bounds, Boolean DurationInState, Average with TreatUncertainAsBad). - `AggregateCttRegressionTests`: direct and live (dispatcher) interval starting on a Bad value, Interpolative with TreatUncertainAsBad, NumberOfTransitions Bad result; the Count test now expects Uncertain values to be counted with TreatUncertainAsBad = false. - `AggregateCalculatorCoverageTests`: an Uncertain raw value at the start with TreatUncertainAsBad is interpolated over. - `Opc.Ua.Server.Tests` and `Opc.Ua.History.Tests` pass on net10.0; net48 builds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-04 19:04:17 +02:00
romanett Claude Opus 5.5 Marc Schier	efad0ddeff	Server subscriptions: fix spec defects in Subscription/MonitoredItem handling found by audit (#4629 ) # Description Fixes defects found in a spec-grounded audit of the server Subscription / MonitoredItem implementation (`src/Opc.Ua.Server/Subscription/`, `NodeManager/MonitoredItem/`, `EventManager`, monitored-item service dispatch and filter validation). Each behaviour change was checked against OPC 10000-4 (§5.13, §5.14, §7.21, §7.22), Part 5 (subscription diagnostics) and Part 9 (ConditionRefresh). Every fix has a regression test. The audit found 56 defects (1 Critical, 3 High, 20 Medium, 32 Low). A follow-up max-effort code review of the fix branch found 27 more issues, mostly introduced by the first round of fixes. All but one are fixed here. ### Most important fixes - Critical — remote hang: an `AggregateFilter` with `processingInterval = NaN` was not revised, and `AggregateCalculator` looped forever while holding the node-manager lock, so one CreateMonitoredItems request blocked the node manager. Non-finite, ≤0 and sub-tick intervals are now revised (Part 4 §5.13.2.1), and the calculator rejects them as a backstop. - Monitored-item limits: new `ServerConfiguration.MaxMonitoredItemCount` / `MaxMonitoredItemsPerSubscription`; Bad_TooManyMonitoredItems is returned when a limit is hit, and ServerCapabilities now report the limits. Both default to `0` (no limit), so existing behaviour is unchanged. - Sampling groups: a client could get one dedicated thread per distinct sampling interval above 1 h; intervals above the last rate group are now rounded, and the loop is async. An item could also end up in two sampling groups and keep being sampled after it was deleted. - Subscription state machine (Part 4 §5.14.1.2): - the lifetime counter resets while a Publish is queued and counts otherwise; - the lifetime and keep-alive stay within `MaxSubscriptionLifetime`; - LatePublishRequestCount is counted once per entry into LATE; - messages held while publishing is disabled are no longer sent, and are not offered by Transfer/Republish either; - ConditionRefresh returns Bad_RefreshInProgress for any refresh already queued or running on the subscription. - Triggering (Part 4 §5.13.1.6): - linked items are triggered by an item still draining a backlog, by the first value after re-enable, and by an item created Disabled; - links are persisted for durable subscriptions; - Disabled/Sampling items follow the publish rules in the shared pipeline, so custom `IMonitoredItem` implementations follow them too. - Queues (Part 4 §5.13.1.5, §7.21): - no duplicate value after a queue-size increase; - EventQueueOverflow events are placed where the events were lost, including on queue shrink; - the duplicate-event check scans the newest entries; - EventQueueOverflowCount and MonitoringQueueOverflowCount are maintained correctly; - changing select clauses rebuilds queued events in the new layout, and is safe on durable queues and on failure. - Filters: - per-clause EventFilterResult instead of rejecting the whole item (Part 4 §7.22.3); - corrected status codes: Bad_DeadbandFilterInvalid, Bad_MonitoredItemFilterInvalid, Bad_EventFilterInvalid, Bad_MonitoringModeInvalid, Bad_TimestampsToReturnInvalid; - aggregate filter in subscriptions: processing interval at least 2× the revised sampling interval, revised StartTime on the requested grid, ServerTimestamp = end of interval (Part 4 §7.22.4). - Lifecycle and races: - expired subscriptions are always cleaned up, even when the session closes during cleanup; - Create/Transfer racing CloseSession returns Bad_SessionClosed; - session diagnostics counts no longer drift; - a large but valid Publish TimeoutHint no longer fails the request; - a ready subscription is served by queued Publish requests instead of going Late; - restored persisted state is validated. - Access: a value change notification is checked against each subscriber's UserAccessLevel, not the writer's. - Redundancy / durable store: - the first-unsent sequence number is mirrored; - sequence numbers are no longer reused after a second failover; - triggering links are stored (definition format 6, written only when links exist, so older replicas can still read link-less subscriptions). ### Behaviour changes reviewers should know - With the default config, publishing intervals above `MaxSubscriptionLifetime / 3` (1,200,000 ms) are revised down, so lifetime ≥ 3×keep-alive fits the configured maximum. - The revised keep-alive is capped at a third of the max lifetime. - Idle subscriptions expire after exactly MaxLifetimeCount cycles without a queued Publish. - A partly invalid EventFilter select list is accepted with per-clause errors. The public `EventFilter.Validate` keeps Status Good if at least one clause is valid; callers that cannot return per-clause results must also check the new `EventFilter.Result.HasSelectClauseErrors`. There is a migration note in `docs/migrate/2.0.x/sessions-subscriptions.md`. - Aggregate processing intervals may be revised higher (≥ 2× sampling interval); aggregate ServerTimestamp is now the interval end. - Sampling intervals above 3,600,000 ms are rounded to multiples of 900,000 ms (default rate groups). ### Public API Additive only: - `ServerConfiguration.MaxMonitoredItemCount` and `MaxMonitoredItemsPerSubscription`; - `EventFilter.Result.HasSelectClauseErrors`; - `IStoredSubscriptionTriggering`, `ISubscriptionRetransmissionSendStateStore`, `SubscriptionRetransmissionState.FirstUnsentSequenceNumber` (optional store capabilities, detected with `as`); - new `EventQueueHandler` constructor overloads. ### Known open items (not in this PR) - Part 4 §5.13.2.1: CreateMonitoredItems should create an item even when the user lacks Read permission and report the error in Publish. This is not done: the create-time RolePermissions check is the only role check on the initial value, so following the spec would leak values until every node manager re-checks per sample. - Sampling loop: it runs on thread-pool threads; node managers with blocking synchronous reads should not use sampling groups heavily. The alternative, a dedicated thread per group, was the thread-exhaustion issue fixed here. - Restarts: a pending QueueSize-1 value and a pending overflow flag are not kept across a restart; that needs persistence interface extensions. - Limit defaults: decide whether the new limits should get finite defaults (currently 0 = unlimited). ## Related Issues - No tracking issue; the findings come from an internal audit of the server subscription code. ## 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. - [ ] 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. ### Local verification (net10.0, all green) \| Suite \| Passed \| Failed \| \|---\|---\|---\| \| Opc.Ua.Server.Tests \| 7963 \| 0 \| \| Opc.Ua.Subscriptions.Tests \| 643 \| 0 \| \| Opc.Ua.Subscriptions.Classic.Tests \| 57 \| 0 \| \| Opc.Ua.Subscriptions.Durable.Tests \| 36 \| 0 \| \| Opc.Ua.Redundancy.Server.Tests \| 847 \| 0 \| \| Opc.Ua.Core.Tests (filter tests) \| 283 \| 0 \| net48 build of `Opc.Ua.Server.Tests`: 0 errors. net48 tests and the full UA.slnx were not run locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>	2026-10-03 05:43:16 +02:00
romanettandClaude Opus 5.5	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
romanettandClaude Opus 5.5	a0d569d756	Fix Server and GDS audit findings, review findings and spec-conformance gaps (#4571 ) ## Summary Fixes the findings of a bug audit of `Opc.Ua.Server`, `Opc.Ua.Gds.Server` and `Opc.Ua.Gds.Client`. Rebased on master after #4516. What went in: - Audit: 93 findings, each checked against the OPC UA specification. - Code review of those fixes: 35 further findings. - Spec grounding of every fix commit: 18 conformance gaps, now closed. Commits are one per finding, named `Fix <ID>`, `Fix review <ID>` or `Fix spec gap <name>`. The IDs refer to the audit report (S = Server areas, G1 = GDS server, G2 = GDS client). Validation: - Each finding was traced in code and checked against OPC 10000 (parts 3, 4, 5, 9, 11, 12, 13, 18, 20). - Every fix commit was checked against the spec text again afterwards: 0 non-conformant, remaining gaps fixed. - Regression tests were added for most fixes. - Suites run on net10.0 before the rebase: Server, GDS, History, Configuration, Features, Core.Security, DI, Redundancy, LDS, InformationModel, PubSub.Server, Client, Sessions and the three Subscriptions suites, all green. The same suites on the rebased branch are still running; results will be posted as a comment. ## Behaviour and user-facing changes ### Secure defaults - `Historize()` / `HistorianBuilder` grant HistoryRead only. History writes need an explicit `historyAccessLevel: HistoryRead \| HistoryWrite`. - File-system mount: - Anonymous sessions can no longer create, delete, move or open files for writing (`Bad_UserAccessDenied`). - UserWritable and UserExecutable now reflect the user. - `AllowAnonymousWrite` restores the old behaviour; the ReferenceServer sample turns it on. - Default PKI root for the hosted server, hosted GDS, hosted LDS and `OpcUaApplicationConfigurationProvider`: - Moved from the shared temp directory to `{LocalApplicationData}/OPC Foundation/{ApplicationName}/pki`, and startup fails if no per-user directory exists. - When certificate stores of an earlier version still exist under temp, a warning names both paths. Existing deployments that relied on the default get a new application certificate (and the hosted GDS a new CA) unless `PkiRoot` is set to the old path. - GDS: - `GdsServerOptions.AutoApprove` defaults to `false`. - The default KeyCredential store grants no roles unless `AuthorizeRoles` allows them. - Certificate requests (StartSigningRequest, StartNewKeyPairRequest, FinishRequest) need an encrypted channel. - Hosted server anonymous logins: - Anonymous is rejected (`Bad_IdentityTokenRejected`) when the default authenticator options disable it. - A custom `AddIdentityAuthenticator<T>()` on its own no longer disables anonymous. - With anonymous disabled and no configured token policies, the endpoint advertises the authenticators' token types instead of Anonymous. - Runtime-added roles: their properties and methods need SecurityAdmin over an encrypted channel, like the well-known roles. - Disabled users: users disabled through Part 18 UserManagement can no longer log in through `RegisterDefaultAuthenticators(IUserDatabase)`. ### GDS server (Part 12) - Certificate requests: - FinishRequest / ReadRequest with another application's RequestId returns `Bad_InvalidArgument`. - Every StartRequest returns a new RequestId. - A CSR without the ApplicationUri is rejected (`Bad_CertificateUriInvalid`). - An abstract or unsupported certificate type is rejected at Start (`Bad_InvalidArgument`). - Private-key password:* no longer persisted in the JSON applications database. A password saved by an earlier version is read into memory once and dropped on the next save. - CheckRevocationStatus: validates offline against the GDS's own trust lists and CRLs. It returns Good for good GDS-issued certificates and `Bad_CertificateInvalid` for input that is not a certificate. - Certificate-group trust lists: - Writes need CertificateAuthorityAdmin; SecurityAdmin and application admins are read-only. - Writes are validated with the Part 4 certificate validation. - AuthorizationService: - The legacy RequestAccessToken applies `AccessControl` and issues the token for the session identity. - StartRequestToken passes the session identity to every identity-aware provider. - Unknown ResourceIds return `Bad_NotFound`. - Expired token requests and refresh tokens are pruned, with at most 10 000 pending requests (`Bad_TooManyOperations`). - KeyCredential: requests with PublicKey + SecurityPolicyUri are rejected with `Bad_SecurityPolicyRejected` (secret encryption is not implemented); a PublicKey without SecurityPolicyUri is rejected with `Bad_InvalidArgument`. - Audit events: access-token audit events no longer contain the password or token data. - RegisterTickets / UnregisterTickets: need the RegistrarAdmin role when called from a session. - Managed application nodes: moved from namespace 0 to their own namespace (`ManagedApplicationsNamespaceUri`); the NodeId is the ApplicationUri, and the mandatory child properties exist. - Hosted GDS: - New `GdsServerOptions.CertificateGroups` (`GdsCertificateGroupOptions`), bindable from configuration. - The mandatory DefaultApplicationGroup is always present. - The default CA subject uses the resolved application name. - JSON applications database: - A corrupt file throws instead of being silently replaced by an empty database. - Saves are atomic and write in place on bind mounts; on net8+ builds they also follow symlinks and keep the Unix file mode. - Issued certificates and trust-list links survive a reload. ### GDS client - Trust-list download: - Reads until an empty chunk (Part 20 short reads). - A size-limit rejection of the final end-of-file read, as sent by released servers, counts as end of file. - Default certificate group: a null group means DefaultApplicationGroup, and vendor groups in other namespaces are no longer mistaken for well-known groups. - Status codes and errors: - Missing directory proxies report `Bad_ServerNotConnected`. - Short or wrongly typed method outputs throw `BadUnexpectedError` / `BadTypeMismatch`. - GdsAccessTokenProvider: no longer caches a failed client task, and one caller's cancellation no longer fails other callers. ### Trust lists, push and ConfigurationFile (Part 12 / Part 20) - AddCertificate: - `IsTrustedCertificate=false` returns `Bad_CertificateInvalid`. - Added certificates are validated (Part 4), and a certificate revoked by a current CRL returns `Bad_CertificateRevoked`. - CloseAndUpdate: validates the uploaded list against its own certificates and CRLs, not the stores it replaces. Validation failures are no longer recorded in the server's Rejected store. - RemoveCertificate: removing a trusted or issuer CA that other certificates still need returns `Bad_CertificateChainIncomplete`. - Handles: - Several read handles may be open at once. - Any open while a write handle exists returns `Bad_NotReadable` / `Bad_NotWritable`. - A write open while any handle is open returns `Bad_NotWritable`. - Handles of closed sessions are released. - Idle handles close after ActivityTimeout (default 60 s). - Status codes: - Read clamps to the remaining bytes and no longer enforces the size limit. - Oversized Write / AddCertificate returns `Bad_RequestTooLarge`. - Unsupported open modes return `Bad_NotSupported`. - ConfigurationFile.CloseAndUpdate: - Empty Targets return `Bad_InvalidArgument`. - The ConfigurationUpdatedAuditEvent is raised with the configuration owner as source. - With the new `IApplicationConfigurationFileTargetProvider`, providers receive the Targets, a bad target returns Uncertain with per-target results, and session-interrupting updates are applied after RestartDelayTime. - Default namespace permissions: namespaces registered after the first permission lookup now get their NamespaceMetadata default RolePermissions and AccessRestrictions (no stale negative cache). ### Sessions, identity and roles (Part 18) - Session timeouts: a failure closing one timed-out session no longer stops the session-timeout monitor. - ChangePassword: an empty old password returns `Bad_IdentityTokenInvalid`. - Removing or disabling a user: closes only that user's UserName-token sessions. - Role configuration changes: roles granted by an `ImpersonateUser` mapping are kept. - X509Subject rules: a certificate subject value containing a quote never matches an X509Subject rule. - `ImpersonateUser` callback: no longer runs under the global session lock. - Session diagnostics: LocaleIds are kept as the client sent them, while null LocaleIds are ignored for translation. ### Subscriptions and monitored items (Part 4) - Acknowledgements: a failing acknowledgement returns a per-entry Bad status instead of failing the whole Publish. - `DiscardedMessageCount`: counts messages dropped from the retransmission queue and unsent overflow messages. - Durable subscriptions: transferring a restored anonymous subscription works (no NullReferenceException). - Transferred subscriptions: sampling and permission checks use the item's current owner session. - Access denied: a monitored item whose user may no longer read the attribute gets a `Bad_UserAccessDenied` notification instead of going silent. Non-Value attributes need Browse permission (RolePermissions needs ReadRolePermissions), and User* attributes are evaluated per subscriber. - ModifyMonitoredItems: a SamplingInterval of -1 uses the subscription's publishing interval. - AggregateFilter priming: needs HistoryRead / ReadHistory permission and is bounded (100 000 values, bounded page size). - Resource leak: event monitored items no longer leak a consumer task per create/delete cycle. ### Node management services (Part 3 / Part 4) - Reference types: AddNodes, AddReferences, DeleteReferences and Browse reject ids that are not ReferenceTypes with `Bad_ReferenceTypeIdInvalid`. AddNodes, AddReferences and DeleteReferences also reject abstract ReferenceTypes; Browse still accepts them (e.g. `HierarchicalReferences` with includeSubtypes). - Concurrent AddNodes: duplicate BrowseNames under one parent get `Bad_BrowseNameDuplicated`. - TranslateBrowsePathsToNodeIds: - Requires Browse permission on intermediate nodes and final targets (RolePermissions and AccessRestrictions). - A long RelativePath no longer rejects the whole request. - SemanticChangeEvent: - Raised once per actual change of a semantic property, whether or not it is monitored. - Also raised for properties with the SemanticChange access-level bit. - Reports the owning node's TypeDefinition as AffectedType. - Localized diagnostics: operation-level text is translated when OperationLocalizedText is requested. ### History and aggregates (Part 11 / Part 13) - DeleteRawModified: an unspecified or reversed range returns `Bad_InvalidTimestampArgument`, and StartTime == EndTime deletes the value at StartTime for every provider. - ReadAtTime fallback: reads only the values around each requested time. A time whose bound is not found within the search limit returns `Bad_BoundNotSupported` instead of failing the request. - ReadProcessed: a ProcessingInterval shorter than one tick returns `Bad_AggregateInvalidInputs`. - Delta / DeltaBounds: widen to the next larger signed type when the result does not fit the source type (e.g. a decreasing unsigned counter, or an Int16 overflow). - Range / Range2: return the source data type. ### Alarms and fluent API (Part 9) - Suppression: an alarm stays suppressed while any suppression group is active, and re-evaluation without a state change does not move TransitionTime. - `MonitorVariable(variable)`: sets SourceNode, SourceName and InputNode to the variable and makes it a ConditionSource (HasCondition to the alarm, HasEventSource from the object). - Fluent input/output arguments: InputArguments / OutputArguments added through the fluent API to a registered method are readable. - Root notifiers: a failed root-notifier registration no longer leaves an orphaned event worker. - State-machine handlers: exceptions are logged at Error. ### File system (Part 20) - Delete / MoveOrCopy: - Only act on direct children of the directory. - A NodeId whose kind does not match the entry returns `Bad_NotFound` (a file NodeId can no longer delete a directory). - Copying a directory into its own subtree returns `Bad_InvalidArgument`. - Locking: - An open file, or a directory containing one, cannot be deleted or moved (`Bad_InvalidState`). - The check is atomic with opens. - While a Delete or Move runs, read opens get `Bad_NotReadable` and write opens `Bad_InvalidState`. - Open: - Reserved mode bits or EraseExisting without Write return `Bad_InvalidArgument`. - Read+Write returns `Bad_NotSupported`. - Read while a writer is open returns `Bad_NotReadable`. - Client `FileSystemErrors`: maps `Bad_NotReadable` to `IOException`. - RuntimeNodeSet: well-formed NodeSets with a stray `Model` element, or two models in one file where one requires the other, now load. ### Server core, hosting and lifecycle - `SessionEstablishmentQueueLimit` > 0: CreateSession/ActivateSession now queue for a permit. The permit is acquired before the request is registered, and a waiting handshake releases its worker through the request-parking policy. - Node manager lifecycle: - Add, reload and remove no longer wait for parked Publish requests. - A Migrate reload racing with CreateMonitoredItems no longer blocks later lifecycle operations. - Late items on retired or removed managers get `Bad_NodeIdUnknown`. - HistoryUpdate: a batch mixing data and event details is bounded by both limits. - Server stop/start: stopping the server disposes the configuration watcher and certificate subscription; starting recreates them. - Monitored-item queue factory: a factory supplied by the caller or the DI container is no longer disposed by the server. - Audit events: each certificate validation error raises its own AuditCertificate event. A failed OpenSecureChannel without a service result is audited as failed. ## New public API (additive) - `IQueuedSessionEstablishmentLimiter` - `StandardServer.OwnsMonitoredItemQueueFactory` (protected virtual) - `DefaultPkiRoot.Get` (Opc.Ua.Configuration) - `TrustList.SetCertificateValidation` - `NamespaceMetadataRegistry.Invalidate` - `IApplicationConfigurationFileTargetProvider` and `ApplicationConfigurationUpdatePlan` - `GdsServerOptions.CertificateGroups` / `GdsCertificateGroupOptions` - `InMemoryKeyCredentialRequestStore.AuthorizeRoles` - `DefaultManagedApplicationsNodeManager.ManagedApplicationsNamespaceUri` - `AuthorizationServiceManager.RequestAccessTokenAsync(..., IUserIdentity?, ...)` - `AllowAnonymousWrite` on the file-system node manager and its factory - `IFileSystemHost.TryBeginMutation` / `EndMutation`, replacing `HasOpenHandles` ## Not fixed / known limitations - KeyCredential: secret encryption with the client's PublicKey (requests needing it are rejected). - CSR SAN restriction: restricting subject / SAN values copied from a CSR (CA policy). - Ticket signatures: ticket signature verification (needs a trusted-signer configuration). - FileType Read+Write open: needs a read/write stream contract in `IFileSystemProvider`. - Custom `IUserDatabase` implementations: the disabled-user check still scans all users per login. The built-in stores do a single lookup. - 64-bit Delta precision: limited by the calculator's double arithmetic. ## Tests with changed expectations Existing tests were updated where they asserted the old behaviour: - File-system status codes - Trust-list handle eviction - `DiscardedMessageCount` - Localized diagnostics bits - Delta on unsigned sources - SourceNode of fluent alarms - ReferenceServer transfer permission test (now expects `Bad_UserAccessDenied` notifications) - `UserDeactivationShutdownReviewTests` (the revoked session is a UserName login) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-09-29 07:26:05 +02:00
romanettandClaude Opus 5	d6356766c5	Resolve the open CTT findings: aggregate fixes, de-DE ApplicationName and CTT configuration (#4545 ) # Description Resolves the open items of `plans/ctt-issues.md` (Needs clarification, Open server findings, CTT project configuration notes). Each item was checked against the OPC 10000 spec text. A server change was made only where the server was wrong, and every server change has a regression test. CTT script and oracle defects are filed in Mantis instead. ### Server changes (`Opc.Ua.Server`) - NumberOfTransitions honours TreatUncertainAsBad. - Part 13 §5.4.3.24 does not count Bad values. §4.2.1.2 makes Uncertain values "equivalent to Bad" when TreatUncertainAsBad is True, and the aggregate definition doesn't override that. - Uncertain values are therefore no longer counted, and no longer used as the previous value, when the setting is True. The now unused `TimeSlice.NonBadEarlyBound` is removed. - DurationInStateZero/NonZero status follows the variable's interpolation. - §5.4.3.2.2: "A region is Uncertain if a region ends in a Bad or Uncertain value and SlopedInterpolation is used." - The calculator always built stepped status regions. So on a sloped variable, the last region stayed Good while TimeAverage2 and Total2 over the same interval were already Uncertain. - The duration value doesn't change. - AnnotationCount returns `Bad_NoData` and sets Partial. - Table 68 requires `Bad_NoData` for intervals before the start or after the end of data. §5.3.3.2 requires the Partial bit for incomplete intervals. - `HistorianDispatcher` now derives the start and end of data from the raw history (read with bounds) plus the annotation timestamps. It passes them to a new `CalculateAnnotationCounts` overload; the old overload is unchanged. - Three existing dispatcher tests change their expectations. For example, a node without any data now returns `Bad_NoData` instead of Good zeros. ### Reference server sample and CTT configuration - `de-DE` ApplicationName. The server advertises `de-DE` in LocaleIdArray, but FindServers and GetEndpoints always returned the en-US name. The sample now keys the ApplicationName and registers en-US and de-DE translations (Part 4 §5.5.2.2 / §5.5.4.2 localeIds). - Subscription limit. `Max Supported Subscriptions` in `samples/UAReferenceServer.ctt.xml` and `MaxSubscriptionCount` in `Ctt.ReferenceServer.Config.xml` go from 100 to 200. Subscription Publish Min 05 `003.js` needs 190 subscriptions. - Alarm Cycle Time. The CTT template's Alarm Cycle Time is 30 s. At 60 s it equalled `RequestedSessionTimeout`, so a Limit/Level CU that starts a run finds its session expired (C40). ### Tests only - AddNodes duplicate BrowseName test: `AddNodeAsync_DuplicateBrowseNameUnderParent_ReturnsBadBrowseNameDuplicatedAsync` never ran. Its items had no TypeDefinition, so its `Assume` always made it inconclusive. The finding it belongs to (AddNodes after a rename returning `BadNodeIdExists`) was already fixed by #4477. - RequestHandle echo: a new test covers the ServiceFault for an oversized argument in a chunked request (already fixed by #4488). - Value-based status: a new test covers the aggregate status with TreatUncertainAsBad = false (already fixed by #4503). ### Not changed - GDS QueryApplications/QueryServers still return records with ServerCapability `NA`. OPC 10000-12 §6.5.10/§6.5.11 says they "shall not", but the CTT GDS scripts expect those records. Implementing the exclusion turned 40 GDS test cases from pass to error, so it waits for the script fix ([Mantis 11458](https://mantis.opcfoundation.org/view.php?id=11458)). - Idle SecureChannels are still closed after `ChannelLifetime`. Keeping them open would be security relevant. The CTT configuration's `ChannelLifetime` of 120000 (#4502) covers the CTT case. ### Mantis issues filed \| Id \| Issue \| \| --- \| --- \| \| C51 \| [11458](https://mantis.opcfoundation.org/view.php?id=11458) GDS scripts expect NA records in query results \| \| C52 \| [11459](https://mantis.opcfoundation.org/view.php?id=11459) A & C Refresh scripts leak subscriptions on the shared alarm session \| \| C53 \| [11460](https://mantis.opcfoundation.org/view.php?id=11460) `StartThreadPublish.js` defaults misspelled properties \| \| U2 \| [11461](https://mantis.opcfoundation.org/view.php?id=11461) Minimum2 oracle on reverse reads \| \| U4 \| [11462](https://mantis.opcfoundation.org/view.php?id=11462) Part 13 clarification: TreatUncertainAsBad and simple bounds (DeltaBounds) \| The A & C `BadSubscriptionIdInvalid` failures (Refresh, Refresh2, Shelving) come from CPU load: with other processes at 100 % CPU, all subscriptions of the CTT alarm session expire within one second. This happens with and without this PR (3 of 3 loaded runs, 0 of 2 idle runs). ### CTT verification CTT 1.05.06, scripts 1.05.513, compared at test-case level against a build of the starting commit: \| Group \| Before \| After \| \| --- \| --- \| --- \| \| Aggregates \| 4,562 error messages \| 4,186. NumberOfTransitions: 12 test cases go from error to pass. AnnotationCount `005-01.js`/`005-02.js` pass. NumberOfTransitions `003-02.js`/`004-02.js` now fail because of C1 ([Mantis 11420](https://mantis.opcfoundation.org/view.php?id=11420)): they request TreatUncertainAsBad = false, but the harness makes the server use its default of true. \| \| Discovery \| 10 warnings \| 0. Find Servers Filter `003.js`/`006.js` and Get Endpoints `002.js` pass. \| \| Subscription \| – \| Publish Min 05 `003.js` goes from warning to pass. \| \| Historical Access, GDS, Node Management \| – \| no new failures \| `plans/ctt-issues.md` is updated: resolved items leave the open sections, and the new Mantis issues are recorded. ## Related Issues - Related to #4479 (CTT configuration notes) - Follows #4502 ## 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. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. Tests run locally on net10.0 after merging master: \| Project \| Filter \| Passed \| \| --- \| --- \| --- \| \| `Opc.Ua.Server.Tests` \| Aggregate, Historian, NodeManagement, Publish, Subscription \| 1,419 \| \| `Opc.Ua.Core.Tests` \| TcpServerChannelBuffer \| 65 \| \| `Opc.Ua.History.Tests` \| Aggregate, Annotation \| 67 \| \| `Opc.Ua.Sessions.Tests` \| discovery locale tests \| 8 \| Before the merge, the whole `Opc.Ua.Sessions.Tests` ClientTest fixture passed (372 passed, 0 failed). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-25 12:30:25 +02:00
Marc SchierandCopilot	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	fa8d45f2cf	Fix server/core validation, lifecycle and protocol regressions (#4477 ) # Description Server/core audit fixes with regression tests. Latest review-fix validation, merge validation and earlier acceptance results are distinguished below. This addresses a 95-finding server/core report. Every finding was reviewed with the OPC UA specification knowledge base or, where the standard does not prescribe a C# implementation, the corresponding ownership/configuration contract. Incorrect or withdrawn subclaims remain qualified rather than being presented as additional fixes. The original checkpoint covers session identity/certificate validation and revocation; certificate/key ownership and transactional updates; TCP reconnect, receive/admission and buffer lifetime; HTTP quotas; subscription admission, durability and pooled payloads; file-operation lifecycle; historian aggregates; fluent/state-machine behavior; and related utilities. It includes targeted regressions and directly related documentation. The approved acceptance cleanup is separate in the commit history: \| Cleanup \| Verified result \| \| --- \| --- \| \| Invalid identity-policy and KeyCredential fixture inputs \| Fixtures explicitly advertise Username and use its generated policy ID. Credential persistence uses valid clear mode inside the existing encrypted admin context. Original permission, lockout and negative encryption assertions remain. \| \| Native GDS lifetime and concurrent connection failures \| Retain ApplicationInstance until client teardown. A deterministic native reproducer proved the test-only channel cap of 10 was exhausted; removing that override restores the existing bounded default of 1000. No global serialization or production quota change. \| \| Stress certificate ownership and close accounting \| Balance two helper references and the fake transport's server-certificate ownership. Replace the invalid event-count bound with exactly-once physical disposal, opens = closes, zero active entries and one logical entry lifetime. \| \| Participant deadlines \| Use the injected TimeProvider with the existing cross-framework WaitAsync overload. Tests pin one tick before, exact expiry, terminal notification, recovery and timely completion. \| \| Reproducibility and platform assertions \| Preserve canonical WoT bytes/hashes, explicit display-culture contracts and exact numeric-vector assertions. Accept Apple's CryptographicException subtype without weakening disposal balance. \| \| Missing user-signature proof \| Reject null/empty proofs before OS crypto. Return BadUserSignatureInvalid per Part 4, 5.7.3.3. None-policy and valid-signature controls remain. A CI-only test nullable-context error was corrected without changing production behavior. \| \| Durable sample restore and shared test storage \| Dequeue waits for actual batch residency instead of treating failed restore completion as success, and restored files close before deletion. Five deterministic regressions failed before the fix. The two fixtures that clear the shared batch directory are isolated; all original 3,000-item assertions remain unchanged. \| ## Latest review follow-up Commit `828856850219518970e8e2f580b8f94b3c1fc748` fixes the deferred robotics disposal reported in the latest review. `RobotIntentNodeManager.DisposeAsync` now joins a shared controller-host drain and then explicitly awaits base node-manager disposal. A controller's bounded shutdown wait may defer its cleanup, but no longer lets the node-manager override finish early. Concurrent callers, including an asynchronous call after synchronous `Dispose`, cannot bypass the drain after the host list has been detached. Synchronous disposal remains nonblocking for deferred work. Deferred initialization failures are logged and retained, and base cleanup failures propagate to asynchronous callers. The existing controller-level bounded disposal contract is unchanged; the node-manager contract is documented in `docs/Robotics.md`. \| Regression requirement \| Evidence \| \| --- \| --- \| \| Wait for deferred hosts; preserve synchronous compatibility and concurrent joins \| `NodeManagerAsyncDisposalWaitsForDeferredHostsAsync`: four cases covering both entry-point orders and zero/positive host shutdown waits. All four failed before the fix. \| \| Wait through base cleanup and observe failures \| `NodeManagerDisposalWaitsForBaseCleanupAndReportsItsFailureAsync`: four cases hold the actual base-resource semaphore, verify pending callers after the host finishes, then check success or the same propagated exception and exactly-once cleanup. \| The focused `IntentHostingCoverageTests` and `IntentControllerHostTests.Dispose` selection passed 30/30 on net10.0 and 30/30 on net48, with no failures or skips. The final net10 solution build passed with zero errors and only the four previously documented CA2249 warnings in unchanged fuzz-test code. No new warnings, analyzer suppressions, dependencies, or weakened assertions were added. The review thread received a commit-specific reply and was explicitly resolved. This verification is scoped to the robotics feedback; the earlier full Subscriptions-run limitation remains recorded in the merge section below. ## Documentation review follow-up (2026-09-14) Commit `f9a9686fcc5d3cfd19cbbc8c2d826e20f5c7e72f` addresses the six follow-up threads on documentation and implementation rationale. Each has an in-thread reply citing the commit and an explicit resolution. Multiline XML documentation now covers the declarations introduced or modified throughout the entire PR, including internal contracts, samples, tests, and public members of related private helpers. A syntax-aware inventory checked 2,031 declarations across all 256 PR C# paths with zero documentation gaps. A supplementary check also covers 75 previously unchanged public members of private helpers involved in the changed code. Token comparison across the conditional-compilation branches proves this follow-up changes no executable C# or test assertions. Project-file changes are rationale comments only. The implementation questions are documented at their source: \| Question \| Resolution \| \| --- \| --- \| \| Certificate assembly friendship \| Server Push configuration reuses the internal explicit-subject CSR helper with the existing key; no public crypto API was added. \| \| Core.Types assembly friendship \| Server builders use internal transition callbacks/revision guards; Server.Tests checks the revision deterministically without creating a public compatibility contract. \| \| Always-single-reader notifications \| One dequeuer preserves event order; optional concurrency fans out within one event and joins before the next event. \| \| Asynchronous disposal completion \| `DisposeAsync` already joins the completion published after admitted operations and all cleanup attempts. The compatibility `Dispose` remains nonblocking to avoid deadlocking an admitted callback against its own drain. \| The requested detailed review disposition follows. It distinguishes actual fixes from confirmed policies and incorrect subclaims rather than counting every comment as a new runtime change. ### Detailed disposition of review 5191795284 The [SDK review](https://github.com/OPCFoundation/UA-.NETStandard/pull/4477#pullrequestreview-5191795284) was submitted against `1c4a9df6c`, before the subsequent merge and feedback fixes. Its 15 inline threads have each received an action-specific reply and an explicit resolution. A fresh GraphQL query verified those replies and resolution states; the fixes in `b8a59990f` remain ancestors of the current branch. \| Review concern \| Action and disposition \| Concrete regression evidence \| \| --- \| --- \| --- \| \| Injected certificate-store providers bypassed \| Fixed. Registration, configuration snapshots, and validation use the injected providers. Explicit custom types remain authoritative; the built-in Directory catch-all is deferred so native X509 paths and later custom providers are not captured. Failed store opens dispose the created store. \| `InjectedProviderResolvesRegisteredAndConfiguredTrustStores` covers default-provider ordering and both registration paths; provider validation/open-failure cases and the unchanged `TrustedCertificateInjectedViaIndependentX509StoreIsObservedAsync` cover real use and independent-store visibility. \| \| Unguarded timer-thread leadership callbacks \| Fixed. Each subscriber's exception is logged independently and cannot suppress subsequent subscribers or fail lease operations. `IsLeader` reads the deadline without invoking application callbacks. \| `ThrowingLeadershipSubscriberDoesNotFailAcquireOrDisposeAsync`, `ExpiryNotifiesOtherSubscribersWhenOneThrowsAsync`, and `IsLeaderDoesNotDispatchExpiryNotificationsAsync` were all demonstrated failing before the fix. \| \| Legacy pending-key provider accepts an unfinishable regenerated CSR \| Fixed by explicit capability validation. A provider implementing only `IPendingCertificateKeyStore` receives `BadNotSupported` before key generation or persistence when regeneration is requested. Existing-key CSR/update remains supported. Regeneration requires matching-only consumption; no unsafe destructive probing was introduced. \| `LegacyPendingProviderRejectsRegenerationBeforeCreatingOrSavingAKeyAsync` checks no key or store mutation; `LegacyPendingProviderCompletesExistingKeySigningAndUpdateAsync` proves the supported flow completes. \| \| Shutdown waits behind user deactivation \| Fixed. Accepted deactivations drain before the master's lifecycle gate is taken. Configuration retains its binding until the shared drain completes, including concurrent shutdown callers and direct master disposal. \| Four cases of `ShutdownDrainsDeactivationOutsideTheSessionClosingGateAsync` exercise real configuration/binding/ServerInternalData/master reentry, retained address space, exact session/subscription closure, and no production cancellation shortcut. \| \| Subclass cleanup runs under the operation-lifetime lock \| Fixed. Cleanup starts outside that lock. A pre-published completion joins all disposers, and only the active asynchronous teardown context may use guarded helpers. Captured contexts lose that privilege when the callback exits. \| `SynchronousOwnedCleanupLetsSamplingWorkerReachClosedAdmissionAsync`, `DisposeCoreCanUseGuardedMembersWithoutReopeningCallerAdmissionAsync`, captured-context rejection, and exact cleanup-failure/ownership assertions cover both synchronous and suspended cleanup. \| \| Discovery requestId-zero chunk read after pool return \| Fixed by the earlier merge. Discovery validation precedes `SaveIntermediateChunk`; no read occurs after that path can return the buffer. The ordering is retained. \| `DiscoveryChannelReadsTheFirstChunkBeforeItIsReturnedAsync` uses a poisoning pool and the requestId-zero path. \| \| T1.8 OPN cleanup misses post-decryption throws \| Fixed by the earlier merge and pinned with new regressions. Both sequence rejection and certificate mismatch release the decrypted body and input ownership. \| `OpenSecureChannelRejectionAfterDecryptionReturnsEveryRentalAsync` checks the exact audit status and pool conservation. Omitting either corresponding release in an isolated snapshot made its case fail; no mutation remains. \| \| T1.2 mount-root protection depends on the provider \| Fixed at the host boundary. Separator-only aliases resolve to the protected root. Traversal, empty/whitespace segments, backslashes, and drive/stream qualifiers are rejected before arbitrary providers are invoked. Valid nested paths and root destinations remain supported. \| A deliberately permissive provider is used by `NonCanonicalObjectPathsCannotDeleteMoveOrCopyAsync` and invalid-target cases; exact zero-call checks are paired with legitimate nested-path/root-destination controls. \| \| Inline trusted/issuer entries now face CRL checks \| Intentional strict behavior, documented and tested. A configured backing store with strict unknown-status rejection requires current revocation information. Explicit-only lists without a store retain their no-store behavior. Actual revocation cannot be suppressed; a matching store entry's policy takes precedence. \| `InlineIssuerWithStoreHonorsStrictUnknownRevocationPolicyAsync`, `InlineIssuerRevocationIsNeverSuppressedAsync`, store-precedence cases, and explicit-only controls preserve the missing-information versus actual-revocation distinction in Part 4 6.1.3. \| \| RSA minimum applied to unspecified ECC identifiers \| Fixed at the application-certificate check. The RSA minimum is compared only with an actual RSA public key. An unspecified ECC identity is not replaced simply because its key size is below an RSA threshold. \| `SilentCertificateCheckPreservesUnspecifiedEccIdentityAsync` checks DER, thumbprint, store contents, and private-key retention; explicit RSA and HTTPS minimum controls remain. \| \| Failed directory refresh wedges corrective mutations \| Fixed. Recoverable automatic pre-refresh failures are logged and leave reconciliation pending, but do not block corrective provider mutations. Explicit refresh errors, capacity admission, caller cancellation, and actual provider denials remain enforced. \| `CorrectiveDeleteRecoversAfterPersistentOverLimitRefreshFailureAsync` converges an externally over-limit directory back to its exact allowed contents. Capacity, cancellation/token, and six refresh-error controls check negative paths. \| \| Timed-transition rejection logs at 10 Hz \| Fixed. Repeated `(state, revision, status)` warnings are deduplicated without changing the retry cadence. A different status or state entry permits another warning. \| `RepeatedTimedRejectionLogsOnceAndRecoversAsync` checks one warning over eleven rejections and success on the next attempt. Status-change and same/different-state reentry controls pin reset behavior. \| \| Bad unsubscribe strands retired-generation bookkeeping \| Fixed. Retired all-event bookkeeping completes even when unsubscribe reports Bad; the first Bad is still returned to the caller. \| Good and BadCommunicationError cases of `RetiredAllEventsDeletionCompletesBookkeepingAndReleasesGenerationAsync` check event/snapshot removal, generation release, and exactly one unsubscribe without finalization replay. \| \| CSR subject parsing and fallback \| Fixed for existing and regenerated keys. Omitted subjects fall back to the actual stored certificate. Regeneration resolves the public certificate's subject before creating a key. Malformed or unresolvable subjects return `BadInvalidArgument` before pending-key mutation. \| Store-only fallback, real-generator null/empty cases, and missing-subject/malformed-DN controls verify the signed CSR, matching fresh pending key, unchanged active identity, and no destructive side effects. \| \| Committed RemoveUser change reported as failed \| Fixed for RemoveUser and ModifyUser. Accepted cleanup is still awaited and close failures remain logged, but cannot replace the committed mutation result. Precommit failures retain their original result and side effects. \| `CommittedUserChangeKeepsResultAndPublishedUsersWhenCloseFailsAsync` checks published Users, Good result, logging, and continued closes. `FailedUserChangeKeepsPublishedUsersAndDoesNotCloseSessionsAsync` checks BadNotFound with no publication or close. \| \| Alleged pooled decoded-request aliasing \| No runtime redesign needed; reviewer rebuttal confirmed. The decoder copies the relevant input data. Misleading ownership comments were corrected rather than introducing a new dispatch model. \| `DecodedRequestRetainsValuesAfterInputBuffersAreReturnedAsync` checks the exact decoded NodeId and ByteString after returned arrays are poisoned. \| The current documentation-only follow-up rechecked 104 Core, 473 Server, and 4 Configuration cases covering these concerns: 581 passed, zero failed or skipped. The final net10 solution build also passed with zero warnings and errors. The complete earlier Core/Server/Configuration and net48 results remain listed separately rather than being presented as rerun current-head acceptance. ## Earlier review follow-up Commit `b8a59990fe2e8e967a12ffd5d9158beed7831143` addresses all 19 review threads. Each thread has an in-thread reply naming this commit and was explicitly resolved after verification; the final paginated query found 19 resolved and zero unresolved threads. The follow-up fixes injected certificate-store selection (including default provider ordering and native X509 paths), actual-RSA minimum enforcement, and CSR capability/subject validation before key mutation. Node-manager teardown uses simpler scoped ownership and drains accepted user revocation outside the master gate; committed user results and retired-event cleanup remain correct when cleanup reports failure. File mutations can recover after failed refresh without bypassing capacity or cancellation, provider paths protect mount roots, and repeated timed rejections no longer flood logs. Leadership subscriber failures are isolated without callbacks from `IsLeader`. Regressions also pin the already-merged OPN/discovery buffer fixes and prove decoded request values do not borrow returned input buffers. The strict inline issuer revocation policy is documented and tested, not weakened. Existing tests were changed only for the reviewed behavior: the incorrect committed-user result was corrected, and inline-issuer/root-alias controls were expanded. ### Review-fix validation \| Suite \| Scope \| Framework \| Passed \| Existing TRX NotExecuted \| \| --- \| --- \| --- \| ---: \| ---: \| \| Core \| Complete project \| net10.0 \| 4,970 \| 88 \| \| Server \| Complete project \| net10.0 \| 6,414 \| 69 \| \| Configuration \| Complete project \| net10.0 \| 231 \| 0 \| \| Core \| Certificate, lease and buffer compatibility slice \| net48 \| 153 \| 0 \| \| Server \| Reviewed lifecycle, user, file, timer and CSR compatibility slice \| net48 \| 692 \| 0 \| \| Configuration \| Complete project \| net48 \| 231 \| 0 \| \| CSR provider regressions \| Final portable-mock follow-up \| net10.0 \| 11 \| 0 \| All runs exited successfully with normal concurrency and leak detection. The Server console reports 5 skips; its TRX additionally records existing assumption/explicit cases, hence the 69-record count above. The Framework build caught two modern-only `ValueTask.CompletedTask` usages in new mock setup; only those expressions changed to the equivalent portable `default(ValueTask)`. The complete modern suites precede that test-only correction; all production files remained byte-identical, and the corrected fixture passed on both targets. The final net10 solution build succeeded with 0 errors and the same 4 CA2249 warnings in unchanged fuzz-test code. Diff checks and changed-line whitespace checks are clean; whole-file formatting still identifies 10 unchanged baseline lines, which were not reformatted outside the feedback scope. The committed 35-file tree was hash-verified against the validation inputs. This is the review-feedback verification scope, not a rerun or sign-off of the historical complete platform/native/soak acceptance campaign below. ## Latest upstream merge Merge `9d9bf679f317164c5b2b90924cd3cdc27a86d451` integrates `master` through `162fc5ebb` (#4486), including the AddNodes child-name index, shared certificate-error reporting, CTT sample-data corrections, and the accompanying regressions and documentation. The CreateSession conflict preserves this branch's certificate ownership, required certificate, application-URI, and channel-certificate checks. The validation result now retains nested errors through `ThrowIfInvalid`, allowing the upstream shared reporting helper to mask trust, policy, and revocation failures consistently without losing audit detail. The new indexed AddNodes regression initially attempted to reuse a deterministic NodeId after renaming its existing node. Renaming frees the BrowseName, not that NodeId. `IndexedBrowseNameReusePreservesExistingNodeIdentityAsync` now proves both contracts: the derived-ID collision leaves the original node intact, then a fresh explicitly requested ID can reuse the old name while the new name remains protected against duplicates. No production identity rule was relaxed. The incoming seeded random-index test uses the existing `UnsecureRandom` wrapper with the same seed and sequence; no new analyzer suppression was added. \| Local merge validation \| Framework \| Passed \| Existing exclusions \| \| --- \| --- \| ---: \| ---: \| \| Core, complete project \| net10.0 \| 5,081 \| 88 \| \| NodeState / BaseInstanceState \| net10.0 \| 579 \| 0 \| \| Server node management, certificate and session-security selection \| net10.0 \| 112 \| 0 \| \| InformationModel node-management services \| net10.0 \| 29 \| 1 \| \| NodeState / BaseInstanceState \| net48 \| 578 \| 0 \| \| Core certificate validation, error reporting and TCP buffers \| net48 \| 111 \| 0 \| \| Server node management, certificate and session-security selection \| net48 \| 112 \| 0 \| All selected runs exited successfully at normal concurrency: 6,602 passing test executions. Modified test assemblies were explicitly rebuilt, and the combined name/identity regression was verified by name in both net10 and net48 results. No skips or weakened assertions were introduced. The final `dotnet build UA.slnx -c Release --no-restore -m:1` with `CustomTestTarget=net10.0` succeeded with zero warnings and zero errors. The resolved source and adjusted tests pass whitespace verification, and their source hashes match the validation inputs. This is merge-specific verification. Earlier review results and the previously reported incomplete full Subscriptions runs below remain scoped to their own commits; they were not reopened or relabeled as passing by this operation. ## Previous upstream merge (2026-09-15 morning) Merge `c4694e20ab089c555af233642042133ff3a03ac8` integrates `master` through `484d6c887`, retaining #4481's shared Like parser/GDS query fixes, #4488's RequestHandle recovery in ServiceFaults, and #4487's expired-session and durable-transfer corrections. Three conflict resolutions preserve both sides: - Alias searches now use the shared `LikePattern` implementation, including upstream malformed-list validation. A timeout overload retains this branch's 100 ms per-alias deadline and `BadTimeout` reporting without keeping a second wildcard translator. The shared matcher's original unlimited overload remains unchanged for existing GDS/ContentFilter callers. New regressions cover bounded prefix/list scans, reuse after timeout, and the alias service result. - Expired activation closes outside the session-manager gate using upstream's once-only timeout claim, audit, and diagnostic accounting. This branch's original-channel and security-policy checks remain in place. - TCP buffer tests retain this branch's ownership, poisoning-pool and post-decryption checks alongside the upstream RequestHandle/fault overload tests; neither set was discarded. The upstream HTTPS binary behavior is intentionally retained: a decoder `ServiceResultException` produces HTTP 200 with an OPC UA ServiceFault carrying the decoder status, response timestamp, and recoverable RequestHandle, rather than the previous HTTP 500 text response. Other exceptions retain their existing HTTP error handling. \| Local validation \| Framework \| Passed \| TRX NotExecuted \| \| --- \| --- \| ---: \| ---: \| \| Core, complete project \| net10.0 \| 5,054 \| 88 \| \| Sessions, complete project \| net10.0 \| 780 \| 610 \| \| Server aliases, expiry/security and subscriptions \| net10.0 \| 288 \| 0 \| \| GDS database, paging and live directory calls \| net10.0 \| 263 \| 0 \| \| Shared Like patterns and bounded evaluation \| net10.0 \| 109 \| 0 \| \| Subscriptions transfer/durability/service selection \| net10.0 \| 23 \| 1 \| \| HTTPS WebApi, complete project \| net10.0 \| 346 \| 0 \| \| InformationModel aliases \| net10.0 \| 54 \| 5 \| \| Shared Like patterns and bounded evaluation \| net48 \| 109 \| 0 \| \| Core fault handles, TCP/HTTPS and filters \| net48 \| 268 \| 0 \| \| Server aliases, expiry/security and subscriptions \| net48 \| 288 \| 0 \| These selected runs exited successfully with normal test concurrency: 7,582 passing test executions. Sessions' console reports 379 skips; the TRX also contains conditional/assumption exclusions, so the table uses the actual 610 NotExecuted records. No skips or weakened assertions were added. The new `EvaluationTimeoutBoundsWorkAndPreservesPatternReuse` and `SharedPatternTimeoutRemainsAnAliasServiceError` tests passed on both targets. Validation limitation: two unfiltered Subscriptions runs did not produce a final result and were stopped, including a retry with a hang guard. They are not counted as passing. The isolated failover fixture passed 2/2, and the transfer/durability/service selection above passed. The stall's cause was not established; no unrelated code or test was changed to conceal it. The final `dotnet build UA.slnx -c Release --no-restore -m:1` with `CustomTestTarget=net10.0` succeeded with 0 warnings and 0 errors. Whitespace verification passed for all hand-reconciled files, and staged source hashes match the validation inputs. These are merge-specific checks, not a new full platform/native/soak acceptance sign-off. Earlier review and acceptance evidence below remains dated to its own commit. ## Previous upstream merge (2026-09-14) Merge `c3e09553e4d1509502d4ba751960d58491bea2cf` integrates `master` through `6313e38a0` (#4478), including the CTT-driven Part 13 aggregate corrections and FindAlias argument validation. The upstream tests and CTT documentation are retained. The single conflict in `AliasNameWildcardMatcher` is reconciled through the branch's shared regex compiler: upstream malformed-pattern rejection and `IsValidPattern` coexist with OPC UA character escaping, whole-string/newline matching, the 100 ms timeout, and service-level error reporting. The upstream ReferenceTypeFilter checks and chronological aggregate behavior are preserved. Local merge verification: \| Suite / selection \| Framework \| Passed \| Existing skips \| \| --- \| --- \| ---: \| ---: \| \| Server aliases, aggregates and historian \| net10.0 \| 1,011 \| 0 \| \| Server aliases, aggregates and historian \| net48 \| 1,011 \| 0 \| \| InformationModel aliases \| net10.0 \| 54 \| 5 \| \| Features aliases \| net10.0 \| 11 \| 0 \| \| Client aliases \| net10.0 \| 28 \| 0 \| \| History aggregate/historical-access consumers \| net10.0 \| 265 \| 0 \| All selected tests passed at normal concurrency. The final net10 solution build passed with 0 errors and 0 warnings. The resolved file's whitespace check and the staged diff check passed. These are merge-specific checks, not a new full platform/native/soak acceptance claim. ## Previous upstream merges Integrates `master` through `8e874d073`, including the Client, Types, Core and source-generator audit fixes in #4471, #4473, #4474 and #4475, plus the event queue-size revision from #4480. The merge commits are `150ea0439` (26 conflict resolutions) and `a0e17c07c` (the base update that arrived during validation). The overlapping fixes are reconciled rather than replacing either branch: - Rejected-certificate retention follows this branch, as explicitly requested: zero means unlimited history; negative values disable new storage while preserving existing backend trimming behavior (Directory clears prior history; shared key/value leaves it intact). The processor and manager policy are not redefined, and documentation/tests describe the distinction. - Preserve owned certificate snapshots and borrowed validation-core lifetimes alongside upstream cache invalidation, store-first CRL checks and `BadCertificateRevocationUnknown` for unsupported revocation lookup. - Combine exactly-once chunk ownership, receive-loop cancellation, stale-epoch rejection and detach-before-adoption with upstream signature-before-padding verification and secret cleanup. - Preserve pooled `HttpClient` per-request configuration together with bounded response reads and deadlines that remain active through body consumption. Regression cases exercise already-used clients and both timeout precedence cases. - Retain the binary-compatible channel-key constructor and normalize omitted profiles; preserve exact profile filtering and upstream binary-first HTTPS/WSS endpoint ordering. - Preserve monotonic lease expiry and stale-confirmation fencing while adding ordered, non-reentrant leadership notifications outside the state lock. ## Validation Local validation of the conflict-resolution merge: \| Suite \| Framework \| Passed \| Existing skips \| \| --- \| --- \| ---: \| ---: \| \| Core \| net10.0 \| 4,940 \| 88 \| \| Client \| net10.0 \| 2,278 \| 0 \| \| Server \| net10.0 \| 6,088 \| 5 \| \| Types \| net10.0 \| 10,394 \| 1 \| \| SourceGeneration.Core \| net10.0 \| 3,940 \| 8 \| \| SourceGeneration \| net10.0 \| 185 \| 0 \| \| Core \| net48 \| 4,855 \| 104 \| After the subsequent four-file #4480 update, the related Server slice passed 811 tests with 5 existing skips, the real durable queue-size cases passed 6/6, and EventManager's 21 cases passed again after analyzer-only expected-array cleanup. The final `dotnet build UA.slnx -c Release` with `CustomTestTarget=net10.0` succeeded: 0 errors and 4 pre-existing CA2249 warnings in unchanged fuzz-test code. Whitespace verification passed for the resolved files and the follow-up. Merge-specific proof includes `RequestTimeoutRemainsActiveWhileReadingTheOpenApiBodyAsync` on already-used shared clients with either the request or HttpClient timeout shorter, the existing ownership/expiry regressions from both branches, and both branches' retention tests adjusted only to the explicitly chosen zero/negative contract. Earlier acceptance at `1c4a9df6c` passed hosted Linux/macOS CI, CodeQL, image builds and the canonical coverage gate (87.15% line, 77.27% branch, 92.14% patch). Both Framework project matrices, all three Windows native suites (183 cases), and the unfiltered net10 stress project (230 cases, including full 60-minute chaos and 30-minute memory soaks) also passed. Those are pre-merge results, not a claim that the full framework/platform/native/soak matrix has been rerun on this merge. The previous acceptance workflow remains separate from this review-feedback operation. Security checks, meaningful assertions, leak detection, default native concurrency, full soak durations and coverage thresholds remain intact. The user-set ready-for-review state is unchanged. No architectural or complete merged-head acceptance sign-off is claimed. ## Related Issues - Fixes #4476 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received.	2026-09-16 04:42:09 +00:00
romanett Claude Opus 5 copilot-swe-agent[bot]romanett	2415503ab9	Close CTT skips in the reference server sample address space, diagnostics EnabledFlag and project settings (#4485 ) Closes #4479 The CTT skipped a number of reference-server test cases because the sample address space, the diagnostics EnabledFlag or the CTT project settings did not provide what the scripts need. This PR fixes the sample data and the server behaviour behind those skips, updates `samples/UAReferenceServer.ctt.xml`, and records the CTT script defects that the newly running cases expose. ## Changes ### Sample address space (`ReferenceServer.NodeSet2.xml`, `ReferenceNodeManager`) \| # \| Skip \| Fix \| \| --- \| --- \| --- \| \| 1 \| Base Info Engineering Units `004.js`: ArrayItemType `EngineeringUnits` have an empty NamespaceUri \| UNECE `mV` (12890) EUInformation, and the axis definitions use `s` (5457219) instead of UnitId 0. \| \| 2 \| Data Access PercentDeadband `006/016/017`: `DataAccess_AnalogType_Float` EURange 0..0 \| EURange 0..100 like the other analog items. \| \| 3 \| Attribute Read `033.js` (19 nodes): second dimension of 3 \| All `Scalar_Static_Arrays2D_` are 5x5 (generated values; the Variant matrix holds scalar Int32 elements). The code-seeded Integer/Number/UInteger matrices are 5x5 too. \| \| 4 \| View Minimum Continuation Point 01 `012.js`, View Basic 2 `018.js`: Method/View nodes need two references of their own NodeClass \| `Methods_Void` HasCause/HasEffect to two methods, `Views_Operations` HasCause/HasEffect to `Views_Engineering`. \| \| 5 \| Base Info Server Capabilities 2 `002.js`: no node without MinimumSamplingInterval \| `DataAccess_DataItem_String` reports `Bad_AttributeIdInvalid` for the optional attribute (Part 3 §5.6.2). A DA item is used because Address Space WriteMask writes every attribute of the static scalars back. \| \| 6 \| Historical Access Read Raw `Err-025.js`: no non-historizing Static Scalar \| New `Scalar_Static_NonHistorizing_Boolean` (HistoryRead access, Historizing = false). See the historian change below. \| \| 10 \| Address Space Atomicity `001.js`: no NonatomicRead node found \| New `AccessRights_AccessAll_NonatomicReadWrite`. The script examines only the first 10000 variables sorted by NodeId string; the 5000 mass variables sort before `Scalar_Static_NonatomicReadWrite`. \| ### Server library - Diagnostics EnabledFlag (item 7, Base Info Diagnostics `018-1/2/3`).* `Server.ServerDiagnostics.EnabledFlag` is writable for SecurityAdmin or ConfigureAdmin over SignAndEncrypt (UserAccessLevel hook, asynchronous write handler); other users get `Bad_UserAccessDenied`. Transitions are serialized, including the node removal. Disabling returns `Bad_NotReadable` for ServerDiagnosticsSummary, its children and the diagnostics arrays, and removes the session and subscription diagnostics nodes (Part 5 §6.3.3). The server keeps the registrations: enabling restores the nodes of the live sessions and subscriptions with the same NodeIds (sessions created while disabled included) and resets the arrays to Good. Counters stay cumulative (required by `018-1.js`). Subscriptions created while diagnostics are disabled still get no diagnostics node. - `DiagnosticsNodeManager.IsDiagnosticsStructureNode` had an inverted NodeId check, so monitored diagnostics nodes never started the scan timer; `OnMonitoringModeChangedAsync` counted every node and replaced a running timer. Both fixed. - Non-historized nodes (item 6). `HistorianDispatcher` raw/modified, processed and at-time reads ask the resolved provider `IsHistorizingAsync` and return `Bad_HistoryOperationUnsupported` for a node it does not historize, instead of the empty result the in-memory historian gave for any node. - Browse order (item 8). `ReferenceDictionary.Find` returned references of a type from hash-based target maps, so a Browse filtered by ReferenceType returned references in a different order than an unfiltered Browse. It now returns them in insertion order (no extra allocation; a single match keeps the fast path). View Basic 2 `015.js` compares both lists by index. The characterization test in `NodeStateReferenceStorageTests` is updated accordingly. ### CTT project settings (`samples/UAReferenceServer.ctt.xml`) \| Setting \| Old \| New \| \| --- \| --- \| --- \| \| `/Server Test/NodeIds/Static/All Profiles/Scalar/Bool` \| `ns=2;s=Scalar_Static_Boolean` \| `ns=2;s=Scalar_Static_NonHistorizing_Boolean` \| \| `/Server Test/NodeIds/References/Has References of a ReferenceType and SubType` \| `ns=2;s=References_HasReferenceTypeAndSubType` \| `i=2253` \| \| `/Server Test/NodeIds/NodeClasses/Object` \| `ns=2;s=Scalar` \| `i=2253` \| `References_HasReferenceTypeAndSubType` loses its HasComponent/HasOrderedComponent references in the source generator; that is tracked in #4484 and not changed here. The Server object has HasComponent and HasAddIn references. ### Decisions - Item 11, Attribute Write StatusCode & Timestamp `003/006/008` (writing ServerTimestamp): unchanged. The stack rejects writes that contain a ServerTimestamp (`NodeState`/`BaseVariableState`), which Part 4 §5.11.4.2 allows (`Bad_WriteNotSupported`); the CTT reports it as "Not Supported", not as an error. Side finding, not changed: the StatusWrite/TimestampWrite AccessLevel bits are not enforced. - Item 6: Part 4 §5.11.3.4 (Table 52) specifies `Bad_HistoryOperationUnsupported`; `Err-025.js` accepts only `Bad_NotSupported`, so the case now runs and reports that as an error. Recorded as a CTT defect in `plans/ctt-issues.md`. - Item 12: removed the stale Historical Access `Err-012.js` entry (fixed by #4026). `plans/ctt-issues.md` adds C32 (`Err-025.js` status code), C33 (Atomicity `001.js`: 10000 cap, operator precedence, substring matching of `UaNodesToIgnore`) and C34 (View Basic 2 `015.js` index comparison), and replaces the Err-025/Err-012 configuration notes with the new settings and the EnabledFlag access requirement. Monitor Basic `039.js` is already C17. The run procedure in `plans/ctt-testing.md` is unchanged. ## Verification CTT 1.05.06, scripts 1.05.513, `ConsoleReferenceServer --ctt -a`, fresh server per conformance unit/group. "master" = `5362ec5e5` with the unchanged project settings. Targeted CUs (skips before → after): Attribute Read `033` Pass; Base Info Diagnostics `018-2`, `018-3` Pass, `018-1` Warning ("Session diagnostics not available" for the session the script creates while diagnostics are disabled); Base Info Engineering Units `001` Inconclusive→Pass, `004` Pass; Base Info Server Capabilities 2 `002` Pass; Data Access PercentDeadband `016`, `017` Warning→Pass; View Basic 2 `005`, `015`, `016`, `018`, `020` Pass; View Minimum Continuation Point 01 `010`, `011` Pass, `012` Warning→Pass; Address Space Atomicity `001` Pass; Monitor Value Change V2 `042` Error→Pass; Historical Access Read Raw `Err-025` runs (CTT status-code defect above); `013` fails on a varying number of results per run (1–12, also 11 on master), the documented continuation-point defect #16. Unchanged: Monitor Basic `039` (CTT script error), Monitor Value Change V2 `020` skip (`Arrays/ByteString` elements shorter than 4 bytes, not part of #4479), Historical Access Delete Value all pass. Saved selection (1,882 cases) per group, errors: Address Space Model 0 → 0, Aggregates 4558 → 4558 and Base Information 19 → 19 (identical messages) measured master (`5362ec5e5`) → this PR; after merging master (#4478) and the review fixes, Aggregates 4210 and Base Information 19; Attribute Services, Base Services, Data Access, Method Services and View Services 0 errors with this PR (0 in the recorded baseline). Tests (net10.0): Opc.Ua.Types.Tests 10395 passed, Opc.Ua.Server.Tests 5561 passed, Opc.Ua.InformationModel.Tests 1082 passed, Opc.Ua.History.Tests 519 passed, Opc.Ua.Client.Tests 2266 passed; no failures. New regression tests: `ReferenceServerTests` (units/ranges, 5x5 matrices, missing MinimumSamplingInterval, non-historizing HistoryRead, NodeClass references, Server object reference subtypes, both nonatomic nodes), `DiagnosticsNodeManagerTests.SetDiagnosticsEnabledAsync_Toggle_HidesAndRestoresDiagnosticsAsync`, `BaseInfoEnabledFlagToggleTests` (admin-only write, Bad_NotReadable while disabled, restore after enable), `ReferenceTableTests.FindReturnsReferencesInInsertionOrder`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-09-15 16:59:04 +00:00
romanett Claude Opus 5 copilot-swe-agent[bot]romanett	93559d06d3	Fix sample A&C refresh and alarm timing found by slow CTT Alarms and Conditions runs (#4482 ) ## Why the CTT Alarms and Conditions group was so slow I measured every A&C CU with a fresh `ConsoleReferenceServer --ctt` (CTT 1.05.06, scripts 1.05.513), read the per-test `ResultNode` timestamps, and added `addLog` timing/counter instrumentation to a copy of the CTT project. \| Cause \| Effect \| \| --- \| --- \| \| CertificateExpiration opens a modal dialog in `initialize.js` ("adjust the clock on the server"), and it also appears in `--hidden` runs \| The CTT never exits. This is why the whole group "did not finish in 40 minutes". \| \| Limit/Level CUs: `initialize.js` idles the CU session for one Alarm Cycle Time (60 s) during the initial capture. `RequestedSessionTimeout` is also 60 s. \| The server correctly closes the session. `LimitHelper` then gets `BadSessionIdInvalid` for its 5 filter subscriptions, and Test_003–006 each run 180 s, about 14 min per CU. \| \| Alarm `Test_002` never gives the AcknowledgeableCondition type a result \| Always runs 3 × cycle (180 s). \| \| Enable `Test_003` needs every alarm type to go active within cycle/10 \| Timed out in 6 of 7 runs, because boolean and analog alarms changed state on different ticks. \| \| Enable `Err_004` answers each event with Disable/Disable/Enable \| Feedback burst, after which the CTT alarm thread sometimes returns no events. Details below. \| \| Comment `Test_001`–`004` compare against `CallHelper.ServerTimeOfCall`, which the CTT never sets (`0001-01-01`) \| Every comment event counts as unrelated, and the tests skip after 3 retries. The server's events carry the expected comment. \| Measured CU by CU, the group took about 87 min plus the hang. With the fixes below, running the whole group in one CTT process without CertificateExpiration takes 18:45 at Alarm Cycle Time 30. The per-CU table is in `plans/ctt-testing.md` §6. ## Server changes - Refresh one subscription per request, not one per Call (`AlarmNodeManager.CallAsync`). The sample answered every ConditionRefresh/ConditionRefresh2 after the first one in a Call with `Bad_RefreshInProgress`, even for other subscriptions, and never forwarded them. So CTT Refresh `Err_004` (10 subscriptions per Call) only ever refreshed one of them. The check is now keyed by SubscriptionId and MonitoredItemId. - Regression tests: `AlarmsAndConditionsRefreshTests.ConditionRefresh[2]OfDifferentSubscriptionsInOneCallSucceedsAsync`. Both fail without the fix. - Boolean alarm source now uses the analog source's steps and limits (`AlarmController`, `ConditionTypeHolder.GetSeverity`). OffNormal, Discrete and SystemOffNormal alarms now change state on the same tick as the limit alarms (5 per second, 70/90/30/10), and the shortest state still lasts 4 s. - With this, Enable `Test_003` passed in 4 s instead of a timeout. Confirm `Test_001` for the boolean types also stopped failing in both group runs. - Test: `AlarmControllerTests.BooleanAndAnalogSourcesChangeStateOnTheSameStep` (fails without the change). - Alarm controllers step on shared interval boundaries (`AlarmController.SetNextInterval`). Scheduling relative to the last step let the two controllers drift up to one 100 ms simulation tick apart, so the boolean events sometimes landed in a later publish and Enable `Test_003` still timed out in 3 of 5 runs. With shared boundaries it passed in 6 of 6 runs. - Test: `AlarmControllerTests.ControllersScheduleTheirStepsOnSharedIntervalBoundaries`. I did not make the simulation faster. Acknowledge, Confirm and Comment tests call methods with the EventId of the event they just received, and a shorter state would make them race the next transition. ## Not reproduced / not a server defect - Refresh `Err_004` `BadSubscriptionIdInvalid` cascade. In the original run the alarm-thread session's subscriptions (15.5 s lifetime) were gone after the script's 30 s wait. Nine further CTT runs passed all 28 Refresh/Refresh2 cases. An in-process replay with a sequential publisher kept every subscription; that replay is what exposed the refresh defect above. - Enable event stall (C43) is inside the CTT. A server build that logs every returned notification message showed that during a stall the server sent 435 and 420 events to the two CTT event items in 173 messages, and the CTT acknowledged all of them, while its alarm-thread buffers returned nothing. It only happens after `Err_004`'s Disable/Enable feedback burst, in 8 of 28 runs. ## Docs - `plans/ctt-testing.md`: new §6 Alarms and Conditions, covering the recommended run, where the time goes, how to instrument a run and the expected result. Also a note that a hidden CTT run can block on dialogs, so always use a timeout. - `plans/ctt-issues.md`: the A&C "open server findings" are replaced with these results, and there are new CTT script defects C39–C44 plus configuration notes (Shelving chattering alarms, Alarm Cycle Time vs session timeout). ## Tests - `Opc.Ua.History.Tests` A&C + AlarmClient: 155 passed. - `Opc.Ua.Server.Tests` `AlarmControllerTests` / `QuickstartsServerUtilitiesTests`: 4 passed. - CTT whole-group runs (27 CUs, 128 cases): 18:45 at cycle 30 and 29:23 at cycle 60 on this branch; 23:43 and 24:21 with only the refresh fix. The spread comes from the Enable stall (C43). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-09-15 14:44:36 +00:00
romanett Claude Opus 5 copilot-swe-agent[bot]romanett	162fc5ebb3	Fix AddNodes scaling and OpenSecureChannel revocation errors found by CTT Monitored Item, Node Management and security runs (#4486 ) # Description Findings from running the CTT conformance groups Monitored Item Services, Node Management Services, Security General and Security User Token against `ConsoleReferenceServer --ctt` (CTT 1.05.06, scripts 1.05.513). The security groups ran without `-a`, against an isolated server PKI built from the CTT's `copyToServer` folders, so the negative certificate tests are meaningful. ## Server fixes ### AddNodes below one parent was quadratic (Node Management Delete Node `Err-002.js`) The CTT adds 15,000 variables below `ns=2;s=CTT` in batches of 5,000 and warned that AddNodes responses took 400–800 ms. For every added node the server searched all children of the parent twice: once in the BrowseName duplicate check in `AsyncCustomNodeManager.AddNodeCoreAsync`, which also copied the children list, and once in the NodeVersion lookup of `FilterChangesByNodeVersion`. - `NodeState.FindChild` now searches children lists with ≥ 64 entries through a BrowseName index kept in a `ConditionalWeakTable`, so nodes with few children get no extra field. - It keeps first-match semantics and is maintained by `AddChild`/`RemoveChild`. - It is dropped on replacement and invalidated when a child is renamed or moves to another parent. - Lists whose children point at another parent (clones) are still searched linearly. - The duplicate check uses `FindChild` instead of copying the list. \| 5,000-node AddNodes batch (in-process, Debug) \| 1st \| 2nd \| 3rd \| \| --- \| --- \| --- \| --- \| \| before \| 769 ms \| 2,083 ms \| 4,011 ms \| \| after \| 141 ms \| 166 ms \| 172 ms \| In the CTT, AddNodes of 5,000 items now answers in 51–72 ms and the warning is gone. ### OpenSecureChannel leaked revocation errors (Security Certificate Validation `002.js`) Part 4 §6.1.3, Table 106: - For the revocation check (`Bad_CertificateRevoked`, `Bad_CertificateIssuerRevoked`), `Bad_SecurityChecksFailed` shall be reported to the client. - For a missing revocation list (`Bad_CertificateRevocationUnknown`, `Bad_CertificateIssuerRevocationUnknown`), it should be. `TcpServerChannel` masked only `Bad_CertificateRevoked`, and `StandardServer.OnApplicationCertificateError` (CreateSession) had its own list that missed `Bad_CertificatePolicyCheckFailed`. Both now use one helper, `CertificateErrorReporting` in Opc.Ua.Core, which also inspects the nested errors of the certificate validator (both paths now throw the validation result with `ThrowIfInvalid`). Only validity period, host name, URI and usage errors keep their own code, and audit events still carry the specific result. The CTT warning is gone. ### Sample data - Monitor Value Change V2 `020.js` skipped because `Scalar_Static_Arrays_ByteString` had a 1-byte and a 3-byte element. The script checks every element, and the simulation only padded elements 0–2. The seed value now has no element shorter than four bytes, and the simulation pads every element. - Monitor Value Change V2 `042.js` got 18 of 19 notifications. The missing item is the Float matrix: its element [1,1] was `-1.03179664E+33`, where adding 1 is below Float precision, so the value really did not change. The server did not drop a notification. The element is now `-1031.79664`. ## CTT results after the fixes \| Group \| Result \| \| --- \| --- \| \| Monitored Item Services (208) \| Monitor Value Change V2 44/44. Remaining: Monitor Basic `039.js` (C17, missing include), `038.js` warning (C32), and 1–2 sporadic timestamp-tolerance warnings that move between test cases from run to run. \| \| Node Management Services (15) \| Only Add Node `Err-008.js` (Mantis 11266). \| \| Security General (314) \| Certificate Validation: 23 pass, 24 not implemented, 3 not-applicable skips. None CreateSession/ActivateSession: all pass. The other 50 CUs are Not Implemented in scripts 1.05.513. \| \| Security User Token (66) \| Anonymous `002.js` (C35) and User Name Password 2 `015.js` (C36) fail because of CTT defects. X509: 18/18 automated cases pass. \| ## CTT defects documented (`plans/ctt-issues.md`) - C32 `038.js` compares a RevisedSamplingInterval of 0 with a project setting instead of the node's MinimumSamplingInterval or the server's MinSupportedSampleRate. - C33 `020.js` requires every ByteString element to be ≥ 4 bytes, but its message says elements 0–2. - C34 `RequestedNewNodeId()` always sends `ns=1`, regardless of `RequestedNodeId_Namespace`, so client NodeIds can't be enabled. - C35 Security User Anonymous picks the `opc.wss` endpoint, which the CTT cannot open (`BadNotSupported`). - C36 `UaEndpointDescription.FindTokenType` rewrites the cached endpoints' token policies, so `015.js` reports PolicyIds as reused. The server's GetEndpoints output was verified compliant (PolicyIds unique per configuration, Mantis 11258). - Evidence added to C9 (042.js) and issue 17 (015.js). `plans/ctt-testing.md` gains how to run the Monitored Item/Node Management groups and a recommended setup for the security groups: isolated PKI from `copyToServer`, no `-a`, PKI restore per part, and waiting for `Server started`. Other findings recorded there: - the Create/DeleteMonitoredItems timing warnings are jitter; - `LoginNameAccessDenied` is not a real user; - `samples\UAReferenceServer.ctt.xml` needs no change (enabling `RequestedNodeId` breaks six Add Node cases because of C34). Note: `plans/ctt-testing.md` and the refreshed `plans/ctt-issues.md` are introduced by #4482. The plans files here are based on #4482's versions, and new entries start at C32 because #4481 uses C19–C31. Please merge after #4482 (or rebase the plans files onto it). ## Tests - `NodeStateChildNameIndexTests` (Opc.Ua.Types.Tests): index semantics (duplicates, remove, rename, replace, re-parent, clone, shrink/grow, 2,000 random operations against a linear search). Also a scan guard: 5,000 misses against 50,000 children take 97 ms with the index and 7,083 ms with it disabled. - `AsyncCustomNodeManagerNodeManagementTests.AddNodeAsync_DuplicateBrowseNameUnderParentWithManyChildren…` - `TcpServerChannelCertificateErrorTests` (Opc.Ua.Core.Tests): masked vs reported certificate errors. - Opc.Ua.Server.Tests 5,553 passed, Opc.Ua.Core.Security.Tests 556 passed, Opc.Ua.Core.Tests (Security + Stack) 3,561 passed, InformationModel NodeManagement 26 passed (net10.0). - Opc.Ua.Types.Tests 10,404 passed and 1 failed: `WotSpecFixtureManifestTests.EveryEmbeddedExampleMatchesItsRecordedHash`. That test hashes WoT assets which are `eol=lf` in the index but CRLF in this worktree's checkout; no WoT file is touched by this PR. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING doc. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added all necessary documentation. - [ ] I ran all tests locally using UA.slnx against .NET Framework and .NET 10 (net10.0 suites listed above only). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>	2026-09-15 10:08:30 +00:00
romanettandClaude Opus 5	36f51cb49d	Add per-type, resource-owning node behaviors to the fluent builder (#4441 ) # Description The fluent builder resolves exactly one node per call. `NodeFromTypeId` is exact-type-only and throws `BadBrowseNameDuplicated` the moment two instances match, and handlers land in single-occupant `NodeState` slots — a second handler for an occupied slot is an error by design. So the fluent surface cannot express four things: - fan-out over the instances of a type, - subtype matching, so a registration for `AnalogItemType` also covers a user's derived type, - composition of two behaviors on one node, - teardown for a behavior that owns something — a semaphore, a file handle, a store, a timer. Three hand-rolled registries already work around exactly this — `EventSourceRegistry` (730 lines), `MonitoredSourceRegistry` (998), `SimulationRegistry` (328) — each with its own register → activate → reconcile → deactivate → dispose loop and its own failure policy, written three times because the slot model had no lifecycle to hang them on. This PR adds that lifecycle and moves the registries onto it. ## The new surface Ten public members. A registration returns an `IAsyncDisposable` that the node manager owns: ```csharp builder.AttachToType<AlarmConditionState>( ObjectTypeIds.AlarmConditionType, async (alarm, ctx, ct) => { MySubscription sub = await Source.SubscribeAsync(alarm, ctx.Lifetime).ConfigureAwait(false); return sub; // released on teardown, in reverse order }); ``` - `AttachToType<TState>(NodeId, attach, NodeAttachOptions?)` — one behavior per matching instance, subtypes included by default. - `Attach<TState>(attach)` on a node builder — one behavior on an already-resolved node. - `Attach(attach)` on the manager builder — manager-scoped, owns no node. - `AcquireWhileMonitored<TValue>(acquire)` — acquire on the first subscriber, release on the last and at shutdown. - `INodeAttachContext`, `NodeAttachOptions`, and `protected ActivateNodeBehaviorsAsync` on `FluentNodeManagerBase`. Guarantees. Behaviors activate deepest node first, and on one node from the base-registered behavior to the derived-registered one; manager-scoped behaviors run last, so they can drive what the node behaviors prepared. Any failure — including cancellation — unwinds every lease recorded so far in exact reverse order, and aggregates cleanup failures with the original rather than replacing it. The engine stays internal. Its vocabulary is not worth freezing into public API, and `INodeAttachContext` + `IAsyncDisposable` is the whole contract a caller needs. Two deliberate decisions worth a reviewer's attention: - Method nodes are excluded from type matching. `MethodState` aliases `MethodDeclarationId` onto `TypeDefinitionId`, so without an explicit filter every method node would match a registration for its declaration — a surprise nobody asked for. Method behavior stays on the call handler. Pinned by a test. - A CLR type mismatch throws `BadTypeMismatch`, naming node, expected and actual type, rather than silently skipping the instance. A whole set of alarms quietly getting no behavior is worse than a startup failure. `Dispose(bool)` is signal-only. Real release belongs in `DeleteAddressSpaceAsync`, which `MasterNodeManager.ShutdownAsync` invokes before it disposes managers on every production path — including the synchronous one, where `StandardServer.Dispose` blocks on `DisposeAsyncCore` → `StopAsync` → `ShutdownAsync` first. Blocking in `Dispose(bool)` would sit under a sync-over-async dispose and `MasterNodeManager`'s semaphore `Wait`. ## The registry migrations `SimulationRegistry` — release moved, plus two latent bugs. `Dispose` cancelled the loops then blocked up to five seconds draining them, from a synchronous dispose. Release now runs on the async teardown path and `Dispose` only trips the token. Loops are driven from the server `TimeProvider` instead of `PeriodicTimer`/`Stopwatch`, so they are testable on a fake clock rather than with sleeps. Fixed alongside: the running loop enumerated the handler list by reference while a late `OnTick` could still mutate it (handlers are now snapshotted, and a late `OnTick` is rejected rather than silently dropped); and a handler throwing `OperationCanceledException` for its own reasons killed the whole loop in silence. `EventSourceRegistry` — ownership only; the reconcile loop stays. It cannot collapse: `OnSubscribeToEventsAsync` runs while the monitored-item semaphore is held and `AddRootNotifierAsync` re-enters `SubscribeToEventsAsync`, so awaiting anything lease-shaped from that hook would self-deadlock. What moves is ownership of the two side effects nothing reversed — the `EventNotifier` bit ORed into the notifier, and the root-notifier registration. A node that already carried the flag is not ours to clear, so only what registration actually changed is recorded and undone. `MonitoredSourceRegistry` — ownership only, and the real leak. Rerouting teardown to `DisposeAsync` turned out to be necessary but not sufficient: that method only clears the item map, cancels the worker and disposes the lock, while the last-subscriber handler runs solely from the reconcile path's `Deactivate` arm — which server shutdown never reaches, because subscriptions dispose their items locally without deleting them. Anything acquired on the first subscriber therefore outlived the server. `ReleaseAsync` now runs those handlers for still-active sources, and `AcquireWhileMonitored` is the demand verb over the pair, with a test that proves release at shutdown with a live subscription. Alarms. Attaching an alarm enabled the condition, promoted the whole notifier chain and registered a root notifier, with no inverse anywhere. `RegisterAlarmEventSource` now reports what it changed and an `AlarmRelease` behavior undoes it. While wiring that up: the parent's `EventNotifier` was being promoted twice — once directly and once inside `RegisterAlarmEventSource` — which left the reversible path with nothing recorded. The redundant promotion is gone. Alarms still work on managers that never opted into the fluent surface; those simply get no automatic release, exactly as before, rather than a hard failure. ## What this deliberately does not do *Registry and alarm setup* stays where it is.** Only release moved. `NodeManagerBuilder.Seal()` is synchronous and is called from sites that never call `CompleteConfigureAsync` — `Isa95NodeManager.cs:1046` seals from inside `private void ConfigureStatusEvents()` — so making behavior activation the sole setup path would silently disable simulations, event sources and alarms in ISA-95, Pump, Generator and RuntimeNodeSet. That same synchrony is what forced the `OnAddressSpaceReadyAsync` hook in #4434 ("`Configure` is a `partial void` and cannot await"), which `Opc.Ua.Di.Server/DiNodeManager.cs` had already invented privately. A separate PR making the fluent configure path async is planned to land first; after it, setup can move onto activation here and both copies of that workaround can go. Also out of scope: incremental attach on runtime node add and release on node removal (the ledger needs real synchronisation first), and the Quickstart `AlarmNodeManager` port that would give the alarm work end-to-end coverage. ## Related Issues No tracking issue yet — this came out of a design review of an earlier behavior-activation prototype, which was rejected for being unreachable from any application and hooked into a loading path rather than the node manager. Happy to open one and link it if maintainers want the ADR on file before review. ## 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. (XML doc comments on every new public member, including why method nodes are excluded and why `Dispose` is signal-only. No `docs/` page — say the word if one is wanted.) - [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 — see Validation. Not run against .NET Framework locally.) - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. ## Validation net10.0: - `UA.slnx` builds clean, 0 errors. - `Opc.Ua.Server.Tests`: 4,984 passed, 0 failed, 5 skipped. - 23 new tests: - 17 on the behavior surface — child-first ordering, base-before-derived on one node, exact-reverse unwind, fan-out over three instances, subtype vs exact-type matching, zero-match diagnostics, declined instances, mid-activation rollback, lifetime-token ordering, second-pass unwind order, drained registrations, method-node exclusion, alarm release, `Publish` release, `Dispose` tripping an activated behavior's lifetime, and the two aggregation contracts (several release failures all surface and do not stop the remaining releases; a rollback that itself fails does not hide the activation failure that triggered it). - 4 on the simulation lifecycle — fake-clock ticking, teardown awaiting an in-flight tick, non-blocking `Dispose`, late-handler rejection. - 2 on `AcquireWhileMonitored` — release on the last subscriber, and release at shutdown with a live subscription. - No new analyzer warnings. Not run locally: .NET Framework targets, and suites outside `Opc.Ua.Server.Tests`. On the advisory coverage gate. The first report put patch coverage at 47.65%, listing whole files as uncovered that these tests exercise directly — `NodeBehaviorRegistry.cs` end to end, for instance. That report is assembled from whichever legs published Cobertura fragments, and the `Server` leg had not. It did point at three paths that genuinely had no test, all now covered: `EventSourceRegistry.ReleaseAsync`, `SignalShutdown` (reachable only via `Dispose` on a manager whose behaviors actually activated), and the two failure-aggregation paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-09 10:42:10 +02:00
romanettandClaude Opus 5	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
Marc SchierandCopilot	7411654c8d	Complete OPC UA Part 11 Historical Access (#4390 ) # Description Completes the OPC UA Part 11 Historical Access work requested by #4387 and the historian framework integration requested by #4400. The conformance/evidence catalog covers all 37 released UACore 1.05 Historical Access profiles: 15 Server facets and 22 Client facets, together with the standard aggregate functions and their conformance units. Server facets are advertised only when the resolved providers expose their required interfaces and capabilities. ## Server and provider support - Completes raw, modified, at-time, processed, annotation, structured, and event history. - Dispatches best-effort, bulk, and transactional updates with aligned statuses, old values, diagnostics, and rollback state. - Supports mixed history-update detail types and batched structured/annotation operations. - Uses composite identities and exclusive cursors for same-timestamp structured, modified, and event values. - Captures live data and events, installs historical configuration objects, and emits typed audit payloads. - Derives aggregate monitoring filters from per-node historian capabilities and rejects unsupported processed aggregates before provider dispatch. - Supports tri-state `Historizing`: set, clear, or preserve provider-owned state. ## Client support - Exposes modified values together with `ModificationInfo`. - Adds event-history reads and event insert/replace/update/delete operations. - Adds structured and annotation batching with strict response validation. - Shares continuation handling and release-on-abandon across read families. - Accepts inherited standard event fields rooted at event subtypes. ## Review hardening - Registers DI-selected historians before node-manager reconciliation and capability publication, while preserving provider ownership. - Uses committed Raft read barriers for linearizable `TryGetAsync` and `ScanAsync`. - Keeps claimed continuation state immutable until a fresh successor is saved, restoring the original after transient failures. - Uses exact-incarnation compare-and-delete so delayed cleanup cannot remove a restored same-ID continuation. - Cleans up indeterminate initial and replacement saves, and propagates shutdown cancellation through cleanup resolution. - Preserves legacy continuation envelopes/cursors and distinct annotation parent/property identities. - Deletes every structured entry at a requested timestamp and uses the complete modified-history ordering tuple. - Shares native/fallback at-time and AnnotationCount calculations, including reverse endpoints and cross-framework fractional intervals. - Primes modified aggregate items before buffered live delivery and deduplicates history/live overlap using provider-specific identities. - Keeps committed Modify results successful while reporting priming failures through the monitored item's notification queue. - Keeps protected-notification state live-only for queues larger than one. Queue size one retains the newest notification without pinning errors or transferring priority during resizing. Durable restore retains ordinary definitions, last values/errors, and raw queues without recreating priority. - Isolates aggregate calculation, modification preparation, and historical/live handoff in `AggregationFilterHandler`, retaining existing provider/factory wiring and supported past-start behavior. The larger-queue protection policy is unchanged pending the open review discussion. - Uses one `IMonitoringFilter` owner for protocol filters or the aggregation handler, eliminating parallel effective-filter storage and retaining the current definition during preparation. The original client filter stays distinct. - Integrates current master, retaining custom monitored-item creation decisions and per-node history callbacks. ## Redundant-server alignment and samples - Adds a protected shared historian with immutable segments, manifest CAS publication, writer fencing, and restart recovery. - Persists portable continuation state before exposing it to clients and transfers ownership during mirrored-session takeover. - Rejects eventual-consistency and active/active multi-writer historian configurations rather than silently degrading. - ReferenceServer and ConsoleReferenceClient demonstrate discovery, paging, raw/modified/at-time/processed reads, annotation/structured CRUD, event capture, and event CRUD. - RedundantServer and RedundantClient demonstrate active/passive history, continuation recovery after replica termination, and post-promotion writes. - Updates Historical Access, High Availability, profile, migration, and sample documentation. ## Compatibility - New 2.0 collection boundaries use `ArrayOf<T>` and public byte payloads use `ByteString`. - Removes the legacy public virtual `SamplingGroupManager.CreateMonitoredItem` / `ModifyMonitoredItem` entry points and protected creation factory, as agreed in review. Migration guidance points to the monitored-item manager and construction-decision APIs. - Retains established persisted formats and current write layouts. Retired interim notification-state formats (sample v2 / shared v4) are rejected, as agreed in review. - Keeps the historian integration compatible with .NET Framework and NativeAOT. ## Validation ### Latest CI source repair `73c619435` forwards the existing cancellation token at both predefined-node creation calls in the PR-added History event fixture. The merged lifecycle overload exposed these two CA2016 errors, which blocked History jobs, the solution build matrix, and CodeQL on GitHub and Azure. The exact compiler failure was reproduced locally; the repaired fixture passes 4/4 integration cases on net10.0 and 4/4 on net48 with clean scoped diagnostics. No assertions, tests, coverage thresholds, or CI configuration were weakened. The repaired History jobs on Linux/macOS and all four Azure solution builds now pass. The Windows all-TFM GitHub job instead lost its hosted runner's connection; no build log was recoverable, and GitHub recorded a runner-loss annotation rather than a compiler/test diagnostic. One unchanged retry was requested. Remaining checks are monitored with `gh pr checks --watch --fail-fast --interval 30`; CI is not yet reported as green. ### Latest base integration `bd52e5fea` merges upstream `master` at `da7412c42`. The lifecycle conflict retains upstream's shared cancellation-aware registration helper and the historian behavior. The hosted-historian fixture forwards its existing token to the new lifecycle overload. GDS project renames and all three AOT test hosts are retained, with their project references resolving after the merge. \| Integrated scope \| net10.0 \| net48 \| \| --- \| --- \| --- \| \| Server lifecycle, fluent authoring/import, historian, monitored-item and hosting regressions \| 732 passed \| 732 passed \| \| NodeState lifecycle regressions \| 216 passed \| 216 passed \| \| Automatically merged Raft client fixtures \| 28 passed \| 28 passed \| No selected tests failed or were skipped. The manually resolved files pass scoped formatting/analyzer checks. One CA1861 warning remains in the incoming `NodeSetImportIntegrationTests.cs`; that file is byte-identical to upstream master and was not rewritten or suppressed during integration. ### Latest feedback follow-up `102954eaa` replaces split filter references with the shared `IMonitoringFilter` contract implemented by protocol filters and `AggregationFilterHandler`. Targeted server regressions pass 103/103 on net10.0 and 103/103 on net48; scoped formatting/analyzers are clean, and the updated XML-documentation inventory has zero gaps across 188 non-generated PR C# files. `93c5b5db8` renames `AggregationFilterHandler`, makes it the owner of the server-revised aggregate filter while preserving the original client filter, and removes the retired interim notification-state readers. Server regressions pass 101/101 and shared-store regressions 64/64, on both net10.0 and net48, with no failures or skips. Runtime-file formatting/analyzers are clean. `5ccf45630` completes the public/internal XML-documentation sweep. The Roslyn inventory finds zero undocumented eligible declarations across all 186 non-generated PR C# files; the 110-file documentation slice changes only XML comments and spacing. `f17765475` applies the three requested Markdown edits. ### Earlier feedback follow-ups `a53950737` removes single-slot notification pinning and extracts aggregate implementation from MonitoredItem. Targeted sampling, aggregate modification, history/live handoff, queue/lifecycle, restore, and serialization cases pass 101/101 on net10.0 and 101/101 on net48, without failures or skips. Scoped formatting/analyzers are clean; the focused review found no significant issues. The final formatted monitored-item fixtures also pass 45/45 on net48. The preceding `b21555568` removes the legacy sampling API bridge and durable notification-priority state. Its targeted Server cases passed 97/97 and shared subscription-store cases passed 64/64 on each framework. The broader evidence below remains pinned to its original tested commit, not attributed to these follow-ups. ### Earlier integrated-implementation validation Validated implementation commit: `3efae597f`. The following commit, `f6dbefb1c`, only adds two explanatory comment lines for the final standards review; it changes no executable code. \| Scope \| net10.0 \| net48 \| \| --- \| --- \| --- \| \| Server \| 5,187 passed; 56 skipped \| Affected surface: 2,160 passed; 1 reproduced baseline failure; 56 skipped \| \| Client \| 2,202 passed; 2 skipped \| 2,204 passed; 5 skipped \| \| History integration \| 519 passed; 21 skipped \| 519 passed; 21 skipped \| \| Redundancy server \| 641 passed \| 641 passed \| \| Redundancy client \| 124 passed \| 124 passed \| \| Core shared-store regression suite \| 23 passed \| 23 passed \| All six affected samples passed non-incremental Release/net10.0 builds with zero warnings and errors: ConsoleReferenceClient, ConsoleReferenceServer, RedundantServer, RedundantClient, MinimalBoilerServer, and PumpDeviceIntegrationServer. The published self-contained win-x64 NativeAOT executable is 116,924,928 bytes and has no CLR header or managed runtime dependency. Direct execution passed HistoryAotTests 3/3 and RaftAotTests 2/2. Final standards and specification reviews found no confirmed outstanding high/medium functional gaps or hard standards violations. The remaining constructor-purpose clarification is addressed in `f6dbefb1c`. ### Reproduced baseline exceptions The net48 hosted-startup timeout also reproduces in a pristine worktree at the previous PR head, `24cf1416d`, in `RegisterPostStartRegistriesWiresHistorianAndAliasStoresOnPlainServerAsync`, with the same subsequent certificate-leak assertion. The corresponding renamed case fails before startup in the reviewed branch as well. This is reported separately from the net48 affected-surface results, not counted as a passing full Server suite. The net48 matrix selected the historian, aggregate, monitored-item, fluent, and continuation surfaces and excluded the Hosting namespace. `DurableDataValueQueueVerifyReferenceBatchingAsync` failed at item 1000 in the net48 affected-surface run and passed its single isolated rerun. The same failure at the same assertion reproduces on pristine `849e16acc`; the test, sample queue, queue factory, and batch persistor are unchanged. The original failure remains in the table rather than being erased by the rerun. ### CI source repairs - `4be7eb8f7` keeps diagnostics/configuration managers out of distributed address-space replication. Those managers inherit the source interface, but their live server state and redundancy metadata must remain replica-local. The ownership regression passes on net10.0 and net48, and all nine redundancy sample CI tests pass locally. - `d17f467b3` completes the sample startup repair: core managers are also replica-local; the startup banner no longer advertises readiness before initialization; the producer starts after distributed services; and native Raft startup no longer forces disruptive extra elections. The full sample suite passed 9/9, with initialized `RedundancySupport=Hot`, and focused Raft/address-space regressions passed 8/8 and 3/3. - `81b0b72be` moves the unchanged distributed-historian NativeAOT scenario into a smaller, always-run companion executable to bound Apple's linker input. All 137 AOT tests remain present; the moved test and helpers are unchanged. GitHub and Azure publish/run both native executables, with separate result directories where they share a job. The companion passed actual Windows NativeAOT execution. Linux and Intel macOS CI passed both executables, but this split alone did not fix ARM64 macOS linking. The remaining historian graph is rooted by ReferenceServer startup, not just the relocated test. No test exclusion or speculative linker setting was added to conceal it. - `8a5842913` isolates the independent MCP test graph into an always-run native host. The existing MCP source and assembly leak hooks are linked unchanged, and evaluated compiler inputs prove exactly one owner for every test source: 133 main + 3 MCP + 1 historian = 137 declarations. All three hosts run on every AOT platform. The main Windows native object is 9.35% smaller; both changed publishes had zero diagnostics. MCP passed 3/3; the identical main executable passed 133/133 on one unfiltered followup after two initial endpoint-discovery connection failures, which remain recorded in the local evidence. - The previous Intel macOS job successfully published and executed its native tests, then failed on artifact-upload DNS (`ENOTFOUND`). No upload gate, shared environment setting, or cache configuration was disabled. The first repair round also encountered the unrelated `MetricsAreEmittedForChannelLifetimeAsync` timing failure on macOS Core. Neither that test nor channel-manager code is changed by this PR, and its assertions remain intact. It passed unchanged on the final repair head. On `d17f467b3`, the Linux and macOS redundancy sample jobs pass; the remaining failure was ARM64 Apple's `too many large addends` assertion. The verified upstream relocation-anchor fix (dotnet/runtime#124721, backport dotnet/runtime#132171) is not present in the shipping ILCompiler 10.0.11. An experimental multi-module probe failed and was rejected, not added to CI. On `8a5842913`, [`aot-macos-latest`](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/34102158680/job/101678934057) successfully publishes and executes all three native hosts on ARM64: 136 main runtime cases, 1 historian case, and 3 MCP cases, with no skips. The previous Apple linker blocker is resolved. Linux NativeAOT also passes. No assertion, test, shared environment setting, cache, or required gate was weakened. The same CI run reports a separate macOS Sessions teardown failure: all 662 executed tests pass, but the global leak assertion detects one undisposed certificate. The MCP partition changes no runtime, Sessions, or test-fixture code. A single unchanged diagnostic rerun is queued once GitHub permits rerunning that job; the failure is not suppressed or counted as green. Other CI checks are still pending. ## Related issues - Fixes #4387 - Fixes #4400 ## Checklist - [ ] I have signed the CLA and read the contributing guide. - [x] I have added regression tests and directly related documentation. - [x] I have retained working reference and redundant-server sample wiring. - [x] Final validation and review evidence is complete, including the baseline exceptions above. - [ ] All required CI checks pass; the fail-fast rerun for `73c619435` is pending. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dbd9527-d818-47a0-984c-fbb1472d3d6b	2026-09-08 11:32:23 +02:00
romanettandClaude Opus 5	1af191ec26	Drop the .Common suffix from the three GDS projects (#4438 ) # Description Every other companion specification in the stack ships as `<Family>`, `<Family>.Client` and `<Family>.Server` — `Opc.Ua.Di`, `Opc.Ua.ISA95`, `Opc.Ua.Positioning`, `Opc.Ua.Robotics`, `Opc.Ua.Vision`, `Opc.Ua.WotCon`, `Opc.Ua.XRegistry`, and `Opc.Ua.Lds.Server` without a family package. GDS was the one exception, carrying 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 `AssemblyName` and `PackageId` disagreed. Nothing that compiles against these assemblies changes a `using` directive; the diff outside the moves is entirely ids, paths and prose. Directories were moved with `git mv`, and every one of the 73 file moves is detected as a rename at 100% similarity (bar the handful that also had a name in their text), so `git log --follow` and blame survive. ## Consumer impact The package ids change, which is visible to anyone consuming the NuGet packages: \| 1.5.378 package \| 2.0 package \| \| --- \| --- \| \| `OPCFoundation.NetStandard.Opc.Ua.Gds.Common` \| `OPCFoundation.NetStandard.Opc.Ua.Gds` \| \| `OPCFoundation.NetStandard.Opc.Ua.Gds.Client.Common` \| `OPCFoundation.NetStandard.Opc.Ua.Gds.Client` \| \| `OPCFoundation.NetStandard.Opc.Ua.Gds.Server.Common` \| `OPCFoundation.NetStandard.Opc.Ua.Gds.Server` \| `docs/migrate/2.0.x/packages.md` gains a renamed-packages section — it is the document that undertakes to cover NuGet renames, and had no GDS entry — and the migration skill's package table gains the two rows a 1.5.378 consumer needs. That is the only hand-written content in this PR; everything else is mechanical. Assembly names follow the package ids, so the section also notes that binding redirects, trimming descriptors and signing manifests naming the files directly have to be updated. ## Build infrastructure Everything naming the projects or their output moves with them: - `UA.slnx` and `tools/MigrationAnalyzer.slnx` (both re-sorted — the project lists are alphabetical, and `Gds` now sorts before `Gds.Client`). - `.azurepipelines/expected-packages.txt` — 3 ids, also re-sorted for the same reason. - `.azurepipelines/signlistDebug.txt` and `signlistRelease.txt` — 18 paths each. Their internal ordering is left alone: that group was already unsorted, and reordering a signing manifest is diff noise. - `samples/Reference/ConsoleReferenceServer/Dockerfile`, the two metapackage nuspecs, the analyzer nuspec's package dependencies, and 8 `.csproj` references across tests, samples and tools. `preview-pack.slnx` is generated from `UA.slnx` by `generate-slnx.ps1`, so there is nothing static to edit. Nothing under `.github/workflows`, and no `.ps1`/`.yml` in `.azurepipelines`, names these projects. The migration analyzer's shim tree mirrors the source project layout by its own documented convention, so `Gds.Client.Common/` becomes `Gds.Client/` and its readme table follows. ## Deliberately not changed Prose naming past work: `plans/23` still records that its deferral happened during the "GDS Client.Common modernization", which is the name that effort had. File paths in the same document were updated. ## Validation `net10.0`: - `UA.slnx`: 0 errors. 14 warnings, all pre-existing CA1861/CA1850/CA1307 in `XRegistry`, `Types.Tests` and `WotCon.Tests` — none in anything this PR touches. - Assemblies build as `Opc.Ua.Gds.dll`, `Opc.Ua.Gds.Client.dll`, `Opc.Ua.Gds.Server.dll`, with no `*.Common.dll` produced anywhere. - `Opc.Ua.Gds.Tests`: 1,116 passed, 0 failed, 54 skipped. - `Opc.Ua.MigrationAnalyzer.Core.Tests`: 10 passed, 3 skipped. - A repo-wide grep for the old names returns only the intentional old → new mappings in the migration docs. ## Checklist - [x] Existing tests pass - [x] No new tests — this is a rename with no behaviour change - [x] Consumer-visible package rename documented in the migration guide 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 20:08:54 +02:00
romanettandClaude Opus 5	3543d7cf6c	Report Executable/UserExecutable for state machine causes (#4368 ) # Description A Part 16 cause is a Method, and whether it may fire depends on the state the machine is in. Part 3 defines `Executable` / `UserExecutable` as exactly that question, and Part 4 has `Call` refuse a non-executable Method with `Bad_NotExecutable`. `FiniteStateMachineState` already knows the answer — `IsCausePermitted(context, causeId, checkUserAccessRights)` — but nothing outside alarm shelving connected the two. Across the library, `OnReadExecutable` was assigned in exactly three places, all in `AlarmConditionState`. Everywhere else a cause method kept the `true` its node was constructed with, so a client could not tell which causes currently apply and found out only by calling one and being refused with `Bad_NotSupported` from `DoCause`. ## What changed `StateMachineBuilder.WithCause` now answers both attributes from `IsCausePermitted` alongside installing the call handler — on the unified builder (via the dispatcher's `InstallCauseMethod`) and on the `Opc.Ua.Server.Fluent` surface reached through `INodeBuilder.AsStateMachine()`, which had the identical gap. `Executable` ignores user rights; `UserExecutable` also runs the machine's `OnCheckUserPermission` callback. Per the issue's note about servers that deliberately keep methods executable and answer at call time, both overloads take `reportExecutable: false` to opt out. `ProgramStateMachineState` wires its own `Start` / `Suspend` / `Resume` / `Halt` / `Reset` in `OnAfterCreate`, so a Program server no longer repeats the boilerplate the class already shipped handlers for. The Boiler quickstart's copy of it is deleted rather than left as an example. *Generator-emitted `StateMachineState` subclasses are left alone. The generator emits no Part 16 `StateTable` / `TransitionTable` / `CauseMappings` overrides — the same gap that makes the DI software-update dispatcher write `CurrentState` directly instead of going through `SetState` (see `docs/SoftwareUpdate.md`) — so there is nothing to derive executability from. A vendor machine from a NodeSet gets its causes through `StateMachineBuilder.For(...).WithCause(...)`, which the first change covers. Worth revisiting if the generator gains table emission. ## Notes for reviewers Two details behind the `ProgramStateMachineState` change: - The five methods are `OptionalPlaceholder` on `ProgramStateMachineType`, so the generated class has no typed properties for them. They are matched on browse name without the namespace index, because a subtype that redeclares them puts them in its own namespace — the Boiler model uses `1:Start`, not `0:Start`. - `NodeState.Create` rebuilds the child list from the type template, so a method added by hand before it is gone by the time `OnAfterCreate` runs. `WireCauseMethods(ISystemContext)` is public for that case. Behaviour change worth flagging: a `WithCause` against a machine whose `CauseMappings` lack that cause now reports the method non-executable where it previously reported executable. Such a call was already failing with `Bad_NotSupported` inside `DoCause`, so it fails earlier with the code Part 4 specifies rather than newly failing — but it is visible to clients that read the attribute. ## Related Issues - Fixes #4363 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. ### Tests New: `StateMachineBuilderCauseExecutableTests` (definition mode, the opt-out, `OnCheckUserPermission` affecting `UserExecutable` only, suspended machines) and `ProgramStateMachineStateTests`, plus two cases added to the existing fluent-builder fixture. All read through `MethodState.ReadAttribute`, so a handler that is installed but never consulted fails the test. Run locally on net10.0**: `Opc.Ua.Server.Tests` (4797), `Opc.Ua.Core.Tests` (4374), `Opc.Ua.Di.Tests`, `Opc.Ua.Features.Tests`, `Opc.Ua.Robotics.Intent.Tests` and the Boiler AOT tests all pass; `UA.slnx` builds with no new warnings. The .NET Framework legs were not run locally — leaving that checklist box to CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-01 18:31:36 +02:00
romanett Claude Opus 5 copilot-swe-agent[bot]romanett marcschier	651beaee1e	Fully support AliasNames in ConsoleReferenceServer (#4012 ) (#4321 ) # Description The CTT AliasName conformance units skip or fail against the ConsoleReferenceServer. `AliasName Base` skips with "No instance of AliasNameType has been found in the address space"; `Category Tags`, `Category Topics` and `Hierarchy` fail. Part 17 support landed in #3779, but aliases live only inside the `IAliasNameStore` — nothing is browsable, no `AliasNameCategoryType` is nested under another, and the Topics aliases point at the Server object rather than at a `PublishedDataSetType` instance. Part 17 §6.2 clients — the CTT among them — discover aliases by browsing, not only by calling `FindAlias`. This PR materializes the store contents as real nodes and fills the three gaps. ### SDK — `Opc.Ua.Server` * `DiagnosticsNodeManager.MaterializeRegisteredAliasNameNodesAsync` (new, opt-in) walks every registered `IAliasNameStore` and creates one `AliasNameType` instance per alias — BrowseName carrying the alias name, `AliasFor` references to its targets, and the inverse `HasAlias` reference on each local target (via `externalReferences`, since targets usually live in another node manager) — plus an `AliasNameCategoryType` instance for every store category the standard NodeSet does not already ship. Categories whose NodeId belongs to another node manager are skipped, so an `AliasNameNodeManager` that also registers its store with the server-wide registry does not get its NodeIds claimed twice. Opt-in: servers that only need the well-known `FindAlias` to answer from their store pay nothing. The nodes are a snapshot taken at address-space creation — later mutations change what `FindAlias` returns but do not add or remove nodes. * Optional Part 17 methods on the well-known categories. The same pass instantiates whatever a category declares through `AliasNameCapabilities`: `FindAliasVerbose` (§6.3.3), `AddAliasesToCategory` (§6.3.4), `DeleteAliasesFromCategory` (§6.3.5) and `LastChange` (§6.3.1). Mutations require a `SecurityAdmin` caller on a `SignAndEncrypt` channel; the gate previously private to `AliasNameNodeManager` moved to `AliasNameMethodDispatcher` so both paths share one implementation. * `LastChange` is now tracked per category rather than only for the standard `Aliases` node, and the duplicated seeding logic collapsed into one helper. ### Reference server * A nested `Devices` category under `TagVariables` holds `Pump1_Status` and `Heater_Power`. `FindAlias` on `TagVariables` still returns all six names through subtree recursion, while `FindAlias` on `Devices` returns exactly two — which is what the Hierarchy unit checks. * `ReferenceServerConfigurationNodeManager` creates three `PublishedDataSet` instances under `PublishedDataSets` (`i=17371`) and the Topics aliases now target them, satisfying the "is a Dataset" check. These are address-space instances only — no PubSub runtime is added, and the CTT only inspects the target's type definition. ## Note for reviewers — the reserved NodeIds The optional children are created at the NodeIds the OPC Foundation reserves for them (`Aliases.FindAliasVerbose` = `i=24054`, `TagVariables.LastChange` = `i=32854`, …). Those identifiers are allocated in `StandardTypes.csv` but are not declared in the ModelDesign and not present in the published NodeSet, because the standard address space does not instantiate optional children. The source generator therefore emits no parent-to-instance mapping and no `MethodIds` constants for them, so `DiagnosticsNodeManager.ReservedChildIds` supplies the nine method ids and the two missing `LastChange` ids explicitly, each traceable to its registry row. The 18 argument-property ids do have generated `VariableIds` constants and are referenced through those. I deliberately did not "fix" this by declaring the children in `tools/Opc.Ua.SourceGeneration.Core/Design/StandardTypes.xml`: that file is a verbatim copy of upstream `UA-ModelCompiler/Design.v105/StandardTypes.xml` (the only differences today are upstream being newer), so a local edit is lost on the next sync and the NodeIds would silently revert to factory-minted ones. `docs/AliasNames.md` records this. Worth raising upstream separately: `StandardTypes.csv` allocates `Aliases_FindAliasVerbose` and siblings while the ModelDesign never declares those children, so the ModelCompiler cannot emit ids it has itself reserved. If that is ever reconciled, `ReservedChildIds` becomes redundant and deletes cleanly. ## Related Issues - Fixes #4012 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. — not done: verified on net8.0 only (Server 4710, InformationModel 1099, Core 4443, plus the client and features alias suites; AOT build clean). Needs a run against net framework and net10 before this leaves draft. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. ### Tests `tests/Opc.Ua.InformationModel.Tests/Aliasname*Tests.cs` mirror the four CTT conformance units and no longer `Assert.Ignore` where the CTT reported failures — the alias suite is 37 tests with no skips. New `AliasnameOptionalMethodsTests` covers the optional methods, their reserved NodeIds, the anonymous-caller denial, an add/find/delete round trip and `LastChange` advancing on mutation. One pre-existing test changed premise: `EnumerateSubCategoriesAsyncOnLeafReturnsEmptyAsync` asserted that `TagVariables` has no sub-categories, which this PR deliberately makes false. It now targets `Topics` (still a leaf) and a new test guards the nesting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>	2026-08-29 18:05:42 +00:00
romanettandCopilot App	d547747e22	Generate ReferenceNodeManager address space from a NodeSet2 model (#4220 ) # Description The CTT/compliance `ReferenceNodeManager` built its ~6,000 node address space entirely in imperative C#. This PR migrates it to a source-generated `[NodeManager]` partial driven by an exported NodeSet2 model (`ReferenceServer.NodeSet2.xml`), mirroring the `MinimalBoilerServer` sample and completing the goal of issue #4204. The address space is now materialized from the model by the source generator. The large orchestration body was kept but converted from create-based to find-instead-of-create: each `Create` primitive now looks up the model-materialized node via `FindPredefinedNode<T>` and re-applies runtime values, DataAccess metadata, and write handlers, rather than constructing nodes from scratch. This keeps the deterministic value seeding and behavior wiring intact while the structure comes from the model. ## Notable details for reviewers - Idempotent reference wiring.* The mesh references in the `References` folder are already present in the model, so the historic manual `AddReference` calls would throw on duplicates. Added a small `AddReferenceIfMissing` helper so those calls are safe. - Non-string NodeIds. The `NodeIds` folder assigns numeric / GUID / opaque NodeIds. The model exports those nodes with their final ids, so find-by-string-path no longer works; added `CreateVariableById(NodeId)` to locate them by their real id. - Generator bug fix (worth a careful look). `ModelDesignExtensions.GetValueRankAsCode` reconstructed `ValueRank` as `dimensions.Length + 1`, so a node with `ArrayDimensions="3,3"` was emitted as `ValueRank = 3`. This affected any NodeSet2 with a fixed multi-dimensional array (rank-3 matrices, `BadTypeMismatch` on whole-matrix writes, wrong decoded dimensions). Corrected to `dimensions.Length` and updated the unit tests that had codified the off-by-one. ## Validation - `ReferenceServerTest`: 44/44 on net10.0 and net48 - `Opc.Ua.SourceGeneration.Core.Tests`: pass on net8.0/net9.0/net10.0/net48 - Integration on net10.0: ComplexTypes, Client, History, Sessions, Subscriptions all pass - Clean build, no new analyzer warnings ## Related Issues - Fixes #4204 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>	2026-08-15 05:22:31 -07:00
Marc SchierandCopilot	1cca744b5c	Retire the last exposed locks: node, browser, server diagnostics and application properties (#4222 ) Closes #4201. Closes #4202. Closes #4203. Closes #4215. Closes #4216. (#4215 and #4216 are @marcschier's originals from the #4188 review; #4202 and #4201 are my later duplicates of the same two findings. All four describe the two changes in section 1 and are closed together.) This PR now carries what was #4221 as well. The two were a stack; they have been unstacked and combined here, and #4221 is closed. Everything below the "Read the server diagnostics" section is the work that was previously reviewed as #4221. --- ## 1. The browse path guards itself, and the last node locks are gone `AsyncCustomNodeManager` kept two `lock (source)` sites around `CreateBrowser`, carrying a NOTE that browser creation was not internally synchronized. `NodeBrowser` still handed its lock to derived types through `protected Lock DataLock`. `NodeState.CreateBrowser` now holds a browse lock across `PopulateBrowser` and `OnPopulateBrowser`, so browser construction on a node is serialized — which is what the external `lock (source)` was actually providing, and what a handler that mutates the node during population (the historian's lazy config installer) depends on. Both `lock (source)` statements and the NOTE are gone. What that lock does not do, stated plainly in the XML docs and in `docs/NodeManagers.md` rather than over-promised: it is not an atomic snapshot across the node's children, notifiers and references. Writers take those collections' own locks, not the browse lock. The browser is a point-in-time copy and each collection is read consistently, but the combination is not a transaction. No `lock` takes a `NodeState` as its sync root any more. That was the last thing keeping `NodeState` from being documentably self-guarding, so the other eleven went with it: `lock (activeNode)`, `lock (target)`, `lock (source.Node)`, `lock (target.Node)`, `lock (notifier)`, `lock (handle.Node)` in `AsyncCustomNodeManager`, and `lock (node)` in `CustomNodeManager2`. Most were a check-then-act pair now served by `NodeState.AddReferenceIfMissing`, which does the check and the insert under the node's own lock. `NodeBrowser.DataLock` is removed. A browser is single-consumer: it belongs to whoever created it, and both server browse paths already serialize on the owning side. The exposed lock only added an inheritance-level locking contract that a derived browser could not reason about. ## 2. Read the server diagnostics under the lock every writer takes `OnUpdateDiagnostics` locked the diagnostics payload rather than the lock every writer takes through `UpdateServerDiagnostics`, so the snapshot a client reads from the `ServerDiagnostics` node was never excluded from a concurrent writer. Both halves matter, and the second is the one that is easy to miss: ```csharp lock (m_diagnosticsLock) // was: lock (ServerDiagnostics) { value = Variant.FromStructure(ServerDiagnostics, copy: true); // was: copy defaulted to false } ``` `copy: true` is what makes it a snapshot. `FromStructure(value, copy: false)` does `new ExtensionObject(value, copy)` — it wraps the live structure without copying it, so the caller would read the fields long after the lock was released and see them change underneath. Swapping the lock alone would have looked like a fix while still excluding nothing. ## 3. Analyzer numbering The browser rule is UA0027, not UA0025. This branch had independently taken UA0025 for `NodeBrowser.DataLock` while #4188 published UA0025 for `ILocalNode.DataLock` and UA0026 for `BaseVariableValue.Lock`. Master's are shipped, so they keep the numbers: \| \| UA0025 \| UA0026 \| UA0027 \| \|---\|---\|---\|---\| \| master (#4188) \| `ILocalNode.DataLock` \| `BaseVariableValue.Lock` \| — \| \| this PR \| `ILocalNode.DataLock` \| `BaseVariableValue.Lock` \| `NodeBrowser.DataLock` \| Both sides had written a different `UA0025Tests.cs`, which arrived as an add/add conflict. They are now split: `UA0025Tests.cs` holds master's `ILocalNode` tests, `UA0027Tests.cs` holds the `NodeBrowser` ones. UA0024's help text also drops its reference to `ReadServerDiagnostics`, which #4193 removed as unused, and names `ISession.ReadDiagnostics` / `ISubscription.ReadDiagnostics` instead — those do still exist. ## 4. Two more locks reachable from outside their owner `ApplicationConfiguration.PropertiesLock` is removed. It returned the properties dictionary itself, so `lock (configuration.PropertiesLock)` was `lock (configuration.Properties)` — the lock was the data it guarded, the same shape as #4203. It had exactly one production consumer (`OAuth2CredentialCollection.Load`, a get-or-add with `ParseExtension` called inside the lock), so it could not simply be deleted. `Properties` is now a `ConcurrentDictionary` — same public type, every operation atomic on its own — and the one multi-operation need gets a member of its own: ```csharp public T GetOrAddProperty<T>(string key, Func<T> valueFactory); ``` It deliberately does not invoke the factory under a lock, so `ParseExtension` now runs outside any critical section. Holding a lock across a caller-supplied callback is exactly the shape that produced the attribute/diagnostics deadlock earlier in this work. The two existing tests asserted only `PropertiesLock is not null`, so they tested nothing but its presence. They are replaced by tests of the contract: factory runs once for a missing key, not at all for a present one, 32 concurrent callers all observe the same instance, null arguments throw. `PropertiesLock` ships in 1.5.378, so this is a breaking removal — migration guide section plus analyzer UA0028. `SharedDefinitionCache` no longer hands out its lock. Three sites in `SharedKeyValueSubscriptionStore` took the nested cache's `Lock` to operate on the cache's own dictionary. Both are private now, behind `ReplaceAll` and `CloneWhere`. `SnapshotCommitLock` stays exposed on purpose: it is a `SemaphoreSlim` sequencing an async commit across `await` points, which `System.Threading.Lock` cannot span, so folding it in would change behaviour rather than hide a lock. ## Verification Merged `master` (`0edfe237a`, including #4234) with no conflicts. - `dotnet build UA.slnx` — 0 warnings, 0 errors - `Opc.Ua.Server.Tests` — 4074 passed - `Opc.Ua.Types.Tests` — 8501 passed - `Opc.Ua.MigrationAnalyzer.Tests` — 151 passed on all five TFMs (138 → 151, the new ones being UA0027 and UA0028) - `Opc.Ua.Core.Tests` — 4249 passed - `Opc.Ua.Redundancy.Server.Tests` — 573 passed ## Note for the reviewer @romanett — your approval was given when this PR's diff was the 7 diagnostics files, because it was measured against `marcschier/node-browse-locks`. Now that the two are combined and the base is `master`, the diff is 23 files, and the extra 16 are the node/browser lock work that was #4221. GitHub kept the approval across the retarget, but it did not cover that code, so please re-review rather than let it ride. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041 Copilot-Session: b3c0e146-5b9e-4d08-a0f6-db3f940484b1 Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63 Copilot-Session: 0ee22c1d-c319-4257-bf78-a5a077a1824a	2026-08-11 22:14:10 +02:00
Alexander Köpke Marc Schier Marc Schier Copilot	0edfe237a6	Bound in-memory historian retention (#4234 ) # Description Bounds raw-data memory usage in `InMemoryHistorianProvider` with a configurable source-timestamp retention window. - Adds `RawDataRetentionPeriod`, defaulting to one hour; `TimeSpan.Zero` restores unbounded retention. - Keeps `MaxSamplesPerNode` as an additional hard cap, with both limits enforced. - Applies retention across bulk, best-effort, update-as-insert, and transactional write paths. - Refreshes the retained high-water timestamp after raw deletions so later out-of-order inserts use the current archive contents. - Explicitly keeps the ReferenceServer conformance historian unbounded because it seeds a multi-hour archive and tests arbitrary historical timestamps. - Documents the default and configuration in `HistoricalAccess.md` and the pump sample README. - Adds regression coverage for defaults, custom and unbounded windows, combined limits, bulk/atomic writes, out-of-order inserts, and both raw deletion APIs. ## Related Issues Addresses the multi-gigabyte memory growth observed when `PumpDeviceIntegrationServer` runs continuously with its in-memory historian enabled. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING guide. - [x] I have added tests that prove the fix is effective. - [x] I have added the necessary documentation. - [x] I have verified the targeted build and analyzer-enabled historian test suite complete without warnings. - [ ] I ran all tests locally using `UA.slnx` against .NET Framework and .NET 10. - [x] I have addressed all PR feedback received. --------- Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-08-11 08:14:10 +02:00
Marc SchierandCopilot	9375a2431b	Retire the IServerInternal bind mutators and freeze the bind phase (#4197 ) ## Stacked on #4193 Base is `marcschier/server-context`, which is itself based on #4188. Review #4188 and #4193 first. GitHub retargets each base automatically as the stack merges. Closes #4184. ## What this does `IServerInternal` published twelve `Set` binding methods plus `CreateServerObjectAsync`, so any holder of the interface could rewire a running server. This takes them off the interface and makes a late bind fail loudly. ## The audit changed the shape of the work The plan this came from said "design a frozen bind phase". One already exists.* `StandardServer` has a seventeen-method `Create` factory seam, and `DependencyInjectionStandardServer` overrides seven of those to resolve from the container: ```csharp protected override IRoleManager CreateRoleManager(IServerInternal server, ApplicationConfiguration configuration) => m_services.GetService<IRoleManager>() ?? base.CreateRoleManager(server, configuration); ``` `StandardServer` then carries each factory result into the datastore: ```csharp ServerInternal.SetRoleManager(CreateRoleManager(m_serverInternal, configuration)); ``` So an integrator already had two supported routes — override the virtual, or register in DI. `Set` on the interface was a third route that bypassed the ordering, and the only one that let a caller rebind a running server. The job was therefore to stop publishing plumbing, not to build a phase. ## Measurement Every call site of the twelve mutators plus `CreateServerObjectAsync`: \| Area \| Count \| Where \| \|---\|---:\|---\| \| `Opc.Ua.Server` \| 11 \| `StandardServer.cs:3729–3844`, one contiguous block \| \| samples \| 1 \| `ReferenceServer.cs:219`, inside a `CreateMasterNodeManager` override \| \| tests \| 20 \| `ServerInternalDataTests` (18), `SubscriptionTests` (2) — all on the concrete `ServerInternalData` \| `SetIdentityRegistry` had zero call sites anywhere. The earlier estimate for this work was 268 files. That number counted files referencing `IServerInternal`, which this change never touches: the interface survives and only the mutators move off it. ## Commits ### `e719a88a` Add the missing `CreateUserManagement` factory seam The one gap, and the reason this had to land first. User management was the only subsystem with no factory seam — the only way to populate `IServerInternal.UserManagement` was `SetUserManagement`, which is what `ReferenceServer` called from inside a `CreateMasterNodeManager` override. Removing the mutator without a replacement would have broken that scenario with no migration path. The DI path at `OpcUaServerBuilderExtensions.cs:1178` builds a `UserNamePasswordAuthenticator` from a registered `IUserManagement` — a different thing entirely that never reaches the Part 18 §5 model on the address space. Purely additive. `ReferenceServer` moves onto the override. The test covers the override seam rather than the DI seam deliberately: registering an `IUserManagement` in the container also switches on the username/password authenticator, so a DI-based test exercises two unrelated paths and fails for reasons unrelated to binding. ### `cbcfda47` Remove `SetIdentityRegistry` Zero call sites, despite a doc comment advertising it as the integrator seam. The supported route has always been `ServerIdentityRegistryExtensions.RegisterDefaultAuthenticators`, which adds authenticators to the default registry rather than replacing it. `IdentityRegistry` is read five times outside `Opc.Ua.Server`, so the default is what every consumer already uses. The analyzer confirmed the reading: with the setter gone, `RCS1170` fired on the property — it is provably never reassigned after construction, so it is now read-only. ### `ea0937a2` Take the bind mutators off `IServerInternal` The mutators stay `public` on `ServerInternalData`, which owns the state. Only the interface loses them. The eleven call sites switch from the `ServerInternal` property to the `m_serverInternal` field, already typed `ServerInternalData?` — the same one-token change `GetStatus` and `EnterState` took in #4193. No test changed. All twenty test call sites already used the concrete type, which is the clearest evidence these were never interface-level API. `CS1574` found six doc comments across `IRoleManager`, `RoleManager`, `IUserManagement`, `UserManagement`, `IServerInternal` and `SessionManager` that pointed integrators at the mutators. They now point at the factory seam. ### `f3c4b9d3` Refuse to bind after the server has started Each bind goes through a guard; `StandardServer` closes the phase once the session manager is in place. A late `Set` throws `ServiceResultException(BadInvalidState)` naming the operation* and pointing at the factory seam — otherwise the failure is indistinguishable from a configuration problem. Rebinding during startup stays legal, which is what let both audit hazards resolve with no behaviour change: - `SetNodeManager` also binds three more managers (`DiagnosticsNodeManager`, `ConfigurationNodeManager`, `CoreNodeManager`). Intentional — they are reached through the master node manager and have no independent lifetime — and now documented rather than silent. - `SetSessionManager` unhooks the previous manager's handlers, which reads as written-for-repeat-calls. Nothing rebinds it, but the freeze does not make the path dead either, since binding twice during startup is still allowed. Left as-is; removing it is a separate question. ## Tests Four new tests on the freeze: the refusal itself across four representative mutators, that the message names the operation, that rebinding stays legal until the phase closes, and that closing twice is harmless. Plus one hosted-server test for the `CreateUserManagement` seam. \| Suite \| net10.0 \| net48 \| \|---\|---\|---\| \| `Opc.Ua.Server.Tests` \| 4033 passed, 1 failed \| 230 passed, 1 failed (filtered) \| \| `Opc.Ua.Redundancy.Server.Tests` \| 573 passed \| — \| \| `Opc.Ua.Redundancy.Kubernetes.Tests` \| 97 passed \| n/a (net8/net10 only) \| The single failure is `ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a local certificate-store problem verified to reproduce unchanged at HEAD with these changes stashed. Not caused by this PR. ## Docs `docs/MigrationGuide.md` gains a table mapping every removed mutator to the `Create` override and the DI registration that replace it, plus the `SetIdentityRegistry` removal with its rationale. ## Net effect across #4193 and this PR `IServerInternal` goes from ~460 lines to ~200. It no longer publishes locks, dead members, or the ability to rewire a running server. ## Reviewer questions - `CompleteBindPhase()` is public on `ServerInternalData` but deliberately not* on `IServerInternal` — only the server owning the startup sequence should decide when binding is over. Reasonable, or should it be `internal`? - `SetSessionManager`'s unhook path is now unreachable in practice but still legal during startup. Leave it, or remove it and assert single-bind for that one subsystem? --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041 Copilot-Session: b3c0e146-5b9e-4d08-a0f6-db3f940484b1 Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63	2026-08-11 00:27:45 +02:00
Marc SchierandCopilot	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	275c3800b5	[Server] Add live NodeManager lifecycle support (#4094 ) # Description Adds supported live add, reload, and remove lifecycle operations for NodeManager instances on a running server. The change introduces a DI-backed `INodeManagerLifecycle` service with direct `StandardServer` access, generation-aware registration handles, copy-on-write routing, transactional prepare/commit/rollback, runtime NodeSet convenience APIs, append-only namespace updates, model and semantic change notifications, additive-compatible complex type priming, and complete teardown behavior. Lifecycle publication is coordinated with requests, sessions, subscriptions, monitored items, continuation points, and shutdown so retired managers cannot remain reachable or be disposed while still in use. Compatible monitored items transfer to replacement generations without a transient bad status. Removed or incompatible Nodes publish `BadNodeIdUnknown` as required by OPC UA Part 4 section 5.8.4.1, retain their MonitoredItem identity, and automatically recover when a compatible Node with the same NodeId returns. Registration is documented end to end in the new [docs/NodeManagerRegistration.md](docs/NodeManagerRegistration.md), which covers compile-time, startup, and runtime registration together with the guarantees for MonitoredItems, Browse continuation points, namespace indexes, DataTypes, and change notifications. ## Review feedback addressed Interface surface reduced, as requested: - `INodeManagerMonitoredItemTracker` folded into `ISubscriptionMonitoredItemLifecycle`. - `INodeManagerMutationCoordinator` and the asynchronous MonitoredItem recovery entry point folded into `IDynamicNodeManagerHost`. - `ISessionClosingRegistry` folded into `IServerInternal`, then replaced by `ISession.IsClosing`. - `ISubscriptionDeletionRegistry` folded into `ISubscriptionManager`, then removed entirely. - `INodeManagerContinuationPointTracker` folded into `ISession`. - The synchronous recovery entry point kept on its own `ISyncNodeManagerMonitoredItemRecovery`, because `AddPredefinedNode` cannot await; it still routes through the asynchronous `INodeManagerMonitoredItemLifecycle` that `AsyncNodeManagerAdapter` exposes. - `IMonitoredItemLifecycle` renamed to `IDetachableMonitoredItem`. Reach into the Subscription area reduced to the minimum the feature needs: - The lifecycle feature itself leaves `SubscriptionManager` almost untouched: two guards that reject a request from a Session that is closing, on `CreateSubscription` and on `TransferSubscriptions`, because it would create state that is about to be torn down. The deletion claim registry, the session closing gates, the closed-session bookkeeping and the deletion restore paths introduced by earlier revisions are gone. Transferring a subscription away from a session that is closing stays allowed, since that is the recovery path the service exists for; the race that exposed is fixed by serialising source-queue closure with the transfer rather than by rejecting it. - Queueing is disabled again for a MonitoredItem with `QueueSize <= 1`, as before this PR. Without a queue the Client is served the last sampled value, so the missing-Node notification simply becomes that value; losing values is expected for a MonitoredItem without queuing. - What remains of the notification is one `DataValue` plus one bool in `DataChangeQueueHandler`, mirroring the existing overflow pair, which stops it being the value discarded when a configured queue is full. - `MonitoredItem` keeps its original publish path, its original queue creation and restore paths, its original triggering behaviour, and no per-epoch bookkeeping. - `IStoredMonitoredItem` carries `IsDeleted` and `IsDetached`, so a restored item reports the state it was stored with rather than one inferred from the last value's StatusCode. Other feedback: - Documented `HostedNodeManagerLifecycle`, `IDynamicNodeManagerHost`, `NodeManagerRoutingTable`, and the `RequestManager` validation and request scopes. - Renamed `ValidateMonitoredItemAsync` to `CanAttachMonitoredItemAsync` and `RestoreMonitoredItemAsync` to `RecoverMonitoredItemAsync`, so the lifecycle API no longer collides with the durable Subscription `RestoreMonitoredItem` path used at server startup. - Removed the `CustomNodeManager2` dependency from the synchronous MonitoredItem recovery path. - `DataChangeQueueHandler` is back to the two constructors 1.5.378 shipped, each taking an optional `TimeProvider`. - Session closing is a one-way latch, and the Session is removed even when teardown throws. - Documented what a custom NodeManager has to implement to take part in live lifecycle operations, and which NodeManager implements `INodeManagerReloadParticipant` today. ## Fixes found during review - The guard that stops a NodeManager lifecycle operation being started from inside a service handler never fired. It was backed by an `AsyncLocal` written inside `async ValidateRequestAsync`, which is invisible to the caller that awaited it, so a lifecycle call from a NodeManager callback waited for its own request and held the lifecycle semaphore forever. The mark is now applied by the service dispatcher, and the drain is bounded so no path can wedge the lifecycle permanently. - `ValidateRequestAsync` is no longer virtual. An override that rejected a request after calling `base` left it registered forever; subclasses now use `OnRequestValidatedAsync`, whose failures complete the request. - A MonitoredItem deleted and disposed while a reload was handing it to the replacement NodeManager could be attached anyway. The item now reserves itself for the hand-over and the transition takes it back if it was disposed meanwhile. - The Overflow bit is recorded and applied when the value is published, as the surrounding design does, rather than written into the value at enqueue time. - MonitoredItem cleanup for a Subscription being deleted no longer passes null entries to `DeleteMonitoredItemsAsync`. - Browse continuation points of a retired NodeManager are persisted and disposed outside the Session lock. ## Open review discussions Three threads are open, replied to and awaiting reviewer decisions: separating service-call dispatch from NodeManager handling in `MasterNodeManager` (tracked as #4098), the request-scoping shape in `StandardServer`, and how much of the async MonitoredItem service implementation should stay in `Subscription`. Issue #4102 records a specification ambiguity found while implementing the missing-Node notification: Part 4 §5.8.4.1 makes it mandatory, but the protected over-capacity queue entry §5.13.1.5 defines applies to `EventQueueOverflowEventType` only, so the specification does not say how it survives a full queue. ## Validation - Full `Opc.Ua.Server.Tests` on net10.0: 3,768 passed, 5 skipped. - Lifecycle, MonitoredItem, Subscription and continuation point suites: 185 passed on net10.0 and 75 on net48. - Server builds complete with zero warnings or errors. - Two failures are pre-existing and unrelated: `DurableDataValueQueueVerifyReferenceBatchingAsync` (flaky `BatchPersistor` file contention, passes in isolation on net10.0 and net48) and `ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync` (fails on a clean tree with these changes stashed). ## Related Issues - Fixes #3993 - Follow-up tracked in #4098	2026-07-28 16:06:28 +02:00
Marc Schier	39991bfea1	Harden managed subscription recreation and reapply (#4065 ) ## Summary - Makes managed subscription recreation generation-safe by quiescing publish ingress, draining in-flight requests, retiring queued messages and acknowledgements from the old server-side generation, then recreating monitored items as one serialized operation. - Exposes `RecreateAsync` on `ISubscription`, rejects recreate/dispose reentrancy from notification callbacks, preserves the active generation when server-side deletion fails, and coordinates disposal with an in-flight recreate. - Aggregates logical subscription recovery state across partitions and adds pending/reapply APIs so monitored items can be deterministically re-created after model or configuration changes. - Initializes the Quickstarts Boiler logger with `NullLogger` so simulation updates remain safe before a logger is assigned. - Keeps the new logical recovery test compatible with net48. ## Review follow-up - Documents the `RecreateAsync` callback-reentrancy and disposal exceptions on the public interface. - Extends generation-retirement coverage with a 1,024-message stale burst and verifies that only ordered current-generation callbacks are delivered. - Proves manager disposal cancels a cancellation-aware operation holding publish quiescence and waits for the gate to release safely. - No runtime timeout or blanket channel drain was added: the quiesced production delegate is internal and cancellation-aware, while unconditional draining could discard or reorder current-generation messages arriving through direct ingress. ## Validation Current head: `edb4a75145c8b03bd03da15eb71327e3bffda091`. Review follow-up validation on the current head: - `Opc.Ua.Client` and `Quickstarts.Servers` build successfully for net472, net48, netstandard2.1, net8.0, net9.0, and net10.0. - focused `SubscriptionTests` and `SubscriptionManagerTests`: 72 passed on net10.0 and 72 passed on net48. The runtime implementation is unchanged from `7b944ac68616e2d6c0595957c65179f86e78a637`, which passed: - net10.0 `Opc.Ua.Client.Tests`: 1,974 passed. - net10.0 `Opc.Ua.Sessions.Tests`: 738 passed, 379 skipped. - net10.0 `Opc.Ua.Subscriptions.Tests`: 601 passed, 6 skipped. - focused net48 managed-subscription tests: 122 passed. - win-x64 Native-AOT publish succeeded; subscription, monitored-item, and Boiler AOT cohorts: 14 passed.	2026-07-23 09:54:26 +02:00
romanett copilot-swe-agent[bot]Marc Schier	537b10c8b3	Fix History User Access Levels and add CTT coverage nodes (#4026 ) # Description Improve Reference Server coverage for OPC UA Compliance Test Tool scenarios and document the latest CTT findings. ## Reference Server coverage - Historize the configured AccessRights nodes and seed them with the same deterministic mixed-quality history as the other test nodes while preserving their access controls. - Add a historian node that intentionally does not support ServerTimestamp. - Add a SelectionListType String instance whose Selections property is String[] as required by Part 5, with localized descriptions and RestrictToList enforcement. - Add an EUR CurrencyUnit property and verify its structured value and binary encoding. - Add a node with explicit read/write base access bits plus NonatomicRead and NonatomicWrite AccessLevelEx flags. - Ensure the first ByteString array elements meet the CTT minimum-length prerequisite. ## Latest CTT findings - Confirm run-23 A&C residuals are CTT re-entrant Read and branch-unaware Retain checks rather than server regressions. - Confirm NumberOfTransitions includes Uncertain values as required by Part 13. - Document the CTT CurrencyUnit decoder dropping a correctly encoded Exponent. ## Related Issues - Follow-up to #3960 and #4002. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. Focused affected tests pass on .NET 10 and .NET Framework 4.8. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com>	2026-07-21 13:52:12 +02:00
Marc SchierandCopilot	3c4661d2bf	Protect certificate and identity secrets in MCP and durable samples (OPC 10000-4 security) (#4043 ) ## Failure MCP auto-accept mutated the shared application certificate manager and accepted every certificate error, so one permissive connection could affect concurrent or later strict sessions. The durable-subscription sample also serialized complete user identity tokens, including user-name passwords and issued bearer tokens. ## Fix - Isolate auto-accept certificate managers/configurations per connection and keep their lifetime with the managed session. - Auto-accept only `BadCertificateUntrusted`; retain strict validation for all other certificate errors and shared sessions. - Version the durable store and reject the previous unsafe format. - Remove user-name passwords before persistence and reject issued-token subscriptions rather than storing bearer credentials. - Add focused tools tests plus solution, project, and durable-subscription docs. ## Reference - OPC 10000-4 Session identity-token and application-certificate security. - Split from #4021. ## Tests - `Opc.Ua.Tools.Tests` Release: 7 passed on net10.0; 4 passed on net48. - `Quickstarts.Servers` Release builds passed on net10.0 and net48 with zero warnings/errors. - `Opc.Ua.Mcp` Release net10.0 build passed with zero warnings/errors. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-21 11:26:25 +02:00
Marc Schier	20d9d65a64	Fix generated configuration namespace resolution (#4019 ) # Description Fix source-generated configuration types whose explicit namespace references constants emitted by the same generator pass. Roslyn cannot resolve those constants while `[DataType]` attributes are analyzed, so the generated XML codec silently fell back to `urn:memorybuffer` / `urn:testdata` while the shipped configuration uses the model namespace URIs. This change: - adds `MODELGEN021` to fail when an explicitly supplied `[DataType.Namespace]` expression cannot be resolved; - uses compile-time-available model namespace URIs for the MemoryBuffer and TestData configuration types; - restores production-shaped XML fixtures and exercises the `UpdateExtension<T>()` / `ParseExtension<T>()` path; - documents the source-generator namespace requirement and migration guidance. Targeted validation passed: - `TypeGeneratorTests`: 17 tests on `net10.0` and `net48`; - `ConfigurationRoundTripTests`: 4 tests on `net10.0` and `net48`. ## Related Issues - Fixes #4014 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received.	2026-07-18 20:15:49 +02:00
Marc Schier Copilot Copilot romanett	84fa8e8906	CTT follow-up: monitored items, alarms, codecs, history, and model validation (#4002 ) # Description Follow-up to #3978 based on the latest OPC UA CTT aggregate, matrix, Historical Access, Attribute, and Node Management runs. ## Aggregate run 16 - Ignore synthetic `BadBoundNotFound` raw-bound markers when feeding aggregate calculators. - Add direct and live historian regression coverage. ## Multi-dimensional Variant matrices - Fix `Variant.Copy()` so rank-2 Variant matrices preserve their payload and dimensions. - Enforce valid Variant matrix dimensions consistently across Binary, JSON, XML stream, and XML parser codecs. - Encoders return `BadEncodingError`; decoders return `BadDecodingError` for malformed matrix dimensions. - Add cross-codec valid/null/empty/malformed tests and a batched ReferenceServer Read regression. - Correct the previous CTT issue attribution: the invalid `Scalar_Static_Arrays2D_Variant` server encoding poisoned the batched CTT Read. ## Node Management AddNodes - Return `BadNodeIdRejected` for unsupported requested-NodeId namespaces. - Validate hierarchical ReferenceType source/target NodeClass constraints. - Document run-18 confirmation that all remaining 15 AddNodes errors are CTT project/script configuration issues. ## Historical Access and Attribute Services run 18 - Reject whitespace in `NumericRange` syntax with `BadIndexRangeInvalid`. - Add HistoryRead tests for operation-level invalid syntax and per-DataValue `BadIndexRangeNoData`. - Document CTT defects: - `012.js` expects `BadIndexRangeNoData` at the operation level instead of on returned DataValues; - `Err-012.js` uses a non-historizing node for an access-denied test; - generic Attribute array helpers omit NodeId[] and StatusCode[] conversion paths. ## Base Information model validation run 19 - Run the CTT main session as `sysadmin` over Basic256Sha256 SignAndEncrypt so SecurityAdmin RolePermissions and EncryptionRequired restrictions are both satisfied without weakening the server model. - Preserve predefined well-known NodeIds while making generated dynamic type and optional-child factories allocate compatible per-instance NodeIds, remap internal references, and update references from the owning subtree. - Materialize dynamic RoleType properties through generated helpers and keep role-child string NodeIds disjoint from numeric RoleManager root IDs. - Add net48/net10 regressions for generator output, NodeId allocation/reference remapping, DI mandatory children, dynamic roles, and low-privilege versus SecurityAdmin Browse visibility. - Document CTT defects for permission-unaware mandatory-child validation, the UA 1.04 reference model used against a UA 1.05 server, stale ServerDiagnostics cache, and the `ConformanceUnits` scalar/array mismatch. ## Related Issues - Follow-up to #3960 and #3978. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING document. - [x] Tests prove the fixes and improve coverage. - [x] Required CTT documentation is updated. - [x] Focused builds/tests introduce no warnings. - [x] Affected tests pass on .NET Framework and .NET 10. - [ ] CI and CodeQL are green. - [ ] All PR feedback is addressed. ## Latest CTT full run and A&C run 20 - Fix Monitor Value Change V2 `036.js`: an ambiguous `DataValue(StatusCode)` call encoded `BadIndexRangeNoData` as a Good value. - Prove all 19 configured matrix monitored items notify when the selected value actually changes; document the independent `042.js` script defects. - Rebase dynamic `CertificateExpired` / `TrustListOutOfDate` alarm roots and descendants, restore standard type declarations and ModellingRules, and expose the cross-node-manager references. - Deliver method-triggered events to every authorized Session, fixing the two-Session Confirm scenario. - Preserve locale-only `LocalizedText` values consistently across Binary, JSON, and XML and align Acknowledge/Confirm comment behavior and tests. - Document the remaining proven CTT defects for TransactionDiagnostics, SemanticChange, History `013`, PolicyId scope, A&C, durable subscriptions, and Subscription Minimum events. ## Base synchronization Merged `master` through `007cfaf90` (`#4010`, repository layout consolidation) and retained the matrix regression tests at their new `tests/` locations. ## Latest validation - Focused merged-tree tests pass on net10 and net48. - `Opc.Ua.Server` and `Quickstarts.Servers` Release builds complete with 0 warnings and 0 errors on net10 and net48. - The existing `DisableEnableViaTypeAndInstanceMethodNoEventsDuringDisabledAsync` timing test took its documented inconclusive timeout path after the layout merge; no test failure was reported. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <romanett98@gmail.com>	2026-07-18 05:01:03 +02:00
Marc Schier	8538f531e7	Update paths for the consolidated repository layout	2026-07-16 06:26:05 +02:00
Marc Schier	7cec2d614d	Move repository files to the consolidated layout	2026-07-16 06:25:54 +02:00