mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-18 22:55:08 +08:00
fix/encoder-stack-depth-json-flake
88 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
0af7ae3a23 |
Update the guides and the MCP server docs for the 2.0.0 release (#4713)
# Description The packages on nuget.org are stable 2.0.0 now. The guides still told readers to use `--prerelease` and preview-only versions, and kept caveats that applied only to 2.0.0-preview.5 and earlier previews. Getting started, Your first information model, and Source-generated data types: - Add the packages without `--prerelease` and name 2.0.0 as the verified version. - With 2.0.0, a client with no server reports `BadNotConnected`, so the troubleshooting row drops the preview.5 `SocketException`. - 2.0.0 generates code for methods that declare their arguments inline, so the tutorial drops the preview.5 workaround note and its troubleshooting row, and says both forms are possible. MCP server (`docs/McpServer.md` and the tool's NuGet README `tools/Opc.Ua.Mcp/McpREADME.md`): - Install and run commands use the stable tool: `dotnet tool exec OPCFoundation.NetStandard.Opc.Ua.Mcp`, `dnx …`, and `dotnet tool install --global …`, without `--prerelease` or `2.0.0-preview.*`. - The stable tool bundles `Opc.Ua.Mcp.Robotics` and `Opc.Ua.Mcp.Vision`, so the note that `vision` and `vision,robotics` need the current source is removed. The docs now state that only the standalone `Mcp.Robotics` and `Mcp.Vision` libraries are preview packages (`2.0.0-preview.200`). - Not changed: `tools/Opc.Ua.Mcp/.mcp/server.json` still declares `"version": "1.0.0"` and lists only the original profiles in its description. ### Validation | Command | Result | | --- | --- | | Getting started Route B, the first-model tutorial, and every troubleshooting row, run exactly as documented against the 2.0.0 packages | Output matches the guides; 0 warnings | | Native AOT publish of the Route B server and client | 0 warnings; the native executables completed the trust flow and connected | | `dotnet tool exec OPCFoundation.NetStandard.Opc.Ua.Mcp -- --version`, `dnx OPCFoundation.NetStandard.Opc.Ua.Mcp -- --version`, `dotnet tool exec "OPCFoundation.NetStandard.Opc.Ua.Mcp@2.0.*" -- --version` | `2.0.0+bc5da1006…` for each | | `dotnet tool install` of `OPCFoundation.NetStandard.Opc.Ua.Mcp` without `--prerelease` (into a temporary tool path) | Installs 2.0.0 | | `dotnet tool exec` of an uncached version with stdin redirected, as an MCP client launches it | Runs without a confirmation prompt | | MCP stdio `initialize` + `tools/list` against the 2.0.0 tool with `--profile vision,robotics`, `vision`, `robotics`, `full` | 64, 26, 42, and 138 tools | | Markdown link, anchor, contents, and fence checks over `docs/` | 0 issues | ## Related Issues - No tracking issue. Follow-up to #4555 after the 2.0.0 release. ## 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
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> |
||
|
|
e78c648295 |
Images CI: publish samples once natively and stop compiling under QEMU (#4679)
# Description
Images CI took 45–100 min per image. The arm64 half of every
`linux/amd64,linux/arm64` build ran `dotnet restore` and `dotnet
publish` in an arm64 SDK container under QEMU. For `refserver`, publish
took 8 min natively for amd64 and 92 min emulated for arm64. On top of
that, every one of the 10 matrix legs recompiled the whole stack.
The output is framework-dependent portable IL, so the compile is now
done once, natively. The image legs only copy files onto the
per-platform runtime base image.
### Dockerfiles (all 10)
- **SDK stage runs on the build platform.** The `build` stage is pinned
to `--platform=$BUILDPLATFORM`. `-p:UseAppHost=false` keeps an x64 app
host out of the arm64 image.
- **New `prebuilt` stage.** It is chosen with a global `ARG
PUBLISH_SOURCE`, which defaults to `build`. Local `docker build` and the
`docker-compose` files are unchanged.
```dockerfile
FROM scratch AS prebuilt
ARG PUBLISH_DIR=artifacts/docker/publish
COPY ${PUBLISH_DIR}/ /app/publish/
FROM ${PUBLISH_SOURCE} AS publish
```
- **Pump: final-stage `RUN`s moved to a native stage.** The `sed` on
`openssl.cnf` and the `mkdir`/`chown`/`chmod` steps now run in a
`$BUILDPLATFORM` `layout` stage. The final stage copies `/layout/` to
`/`, so `/app` and `/diag` keep the `$APP_UID:0` owner and `g=u` mode
they had before. `openssl.cnf` therefore comes from the build-platform
base image. It is a plain config file, so it is expected to match
between architectures.
### `docker-image.yml`
- **Single image list.** The image list lives once, in `env.IMAGES`
(image, project, Dockerfile). `discover` emits it as the matrix.
- **New `publish-samples` job.** It runs `dotnet publish` for each
sample sequentially, so shared libraries compile once. It uses the same
options and NBGV version properties as the Dockerfiles and uploads the
output as the `docker-publish` artifact.
- **`build-and-push-image` legs.** Each leg:
- downloads the artifact;
- builds with `PUBLISH_SOURCE=prebuilt` and
`PUBLISH_DIR=artifacts/docker/publish/<image>`;
- no longer uses `setup-qemu-action`;
- is named `build-and-push-image (<image>)`.
- **`images summary`.** It now also fails when `publish-samples` fails.
### Docs
- `ContainerReferenceServer.md` describes the publish-once / copy-only
flow and the build arguments.
- `DeveloperGuide.md` mentions the `publish-samples` job in the Images
CI section.
Expected: one native publish of about 7 min, then 10 parallel
copy-and-push legs of a few minutes each. Not yet confirmed on CI: the
sandbox could not pull MCR base images, so the prebuilt multi-platform
builds were checked against a Debian-based stand-in base, and the
build-from-source path was not built.
## Related Issues
## Checklist
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [ ] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
|
||
|
|
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> |
||
|
|
b25c1f6ad4 |
Asset Management Basics: the OPC 10000-110 companion SDK (#4662)
# Description Stacked on #4650. Until that is merged, this PR also shows its commits; the six commits of this PR start after `fix(server): keep conformance unit aggregation idempotent and ordered with registrations`. This adds OPC 10000-110 Asset Management Basics 1.01.1 (`http://opcfoundation.org/UA/AMB/`) as a companion SDK, in the pattern of the DI, Machinery, Pumps and Scales packages. The tracking issue #4649 describes the scope, the design and the open questions. In short: | Package | Content | |---|---| | `Opc.Ua.AMB` | the model generated by the source generator from the unmodified NodeSet, plus the contracts server and client share (`AssetFaultSeverity`, `AssetLocationKind`, `MaintenanceStateKind`, `DocumentationLinkProperties`) | | `Opc.Ua.AMB.Server` | a sidecar node manager next to the owner of the Device Integration address space (`AddOpcUaDi`, `AddMachinery`, `AddPumps` or `AddScales`), the asset registry `IAssetManagement` with a fluent builder, `AddAssetManagement()` hosting, memory and file system stores, the AMB conformance units and server facet | | `Opc.Ua.AMB.Client` | `AmbClient`: discovery, identification, health alarm and maintenance streams, documentation links, locations, structure and snapshots, plus `AddAssetManagementClient()` | **Sidecar.** An asset stays in the node manager that owns it. Everything AMB adds to it (`DeviceHealth`, health alarms, maintenance conditions, the DocumentationLinks AddIn, AssetId and the other properties) is created there. The AMB node manager serves the AMB model, the alias categories `Assets`, `AssetsByProductInstanceUri` and `AssetsByAssetId` below `0:Aliases`, the location hierarchies below `0:Locations` and the server-specific alarm and condition types. Owners register assets with the shared registry from their setup, in either startup order. **Implemented conformance units** (Table 54): Asset Identification, Configurable Asset Identification, Discovery by ProductInstanceUri and by AssetId, Health Status Base, Alarms, Root Causes and Alarm Categories, Version Information, Operation Counters (inspected only; AMB defines no counters), DocumentationLinks Base, Edit Base and Edit Advanced, Requirements, Capabilities, Classification, Current and Future Maintenance Activities, Local Time, the three location properties and both location object hierarchies, Sub-assets and Asset relations. The two history units (Health Tracking, Past Maintenance Activities) are not part of this PR. `docs/AMB.md` has the matrix with the rule the server applies for each unit and whether a test checks it over a session. **Dependency on #4650.** The health alarms and maintenance conditions are created through `CreateAlarm<TState>`, and the conformance units follow the registered assets through the recomputation in `ConformanceUnitsManager`. ## Changes Six commits, each building on its own: 1. **`feat(amb)`: model package** (`src/Opc.Ua.AMB/`, about 2.4k lines, of which the NodeSet and the two identifier tables are 1.6k). - The NodeSet is the publication from `UA-Nodeset/latest/AMB`, byte for byte (git blob `21f9475b646e9c5508171277ffe5b74edbeb0a58`). `.gitattributes` checks it out with LF endings, because a test pins its SHA-256. - The published identifier table is vendored unchanged; a derived table pins the symbol-to-NodeId mapping, and the generator fails the build when they disagree. `tools/nodesets/README.md` documents the one new kind of renamed symbol (top-level instances below `0:Aliases` and `0:Locations`). 2. **`feat(amb)`: server package** (`src/Opc.Ua.AMB.Server/`, about 10.7k lines). Node manager, registry and builders, discovery, health, maintenance (`MaintenanceEventStateMachine` with all transitions), documentation links, locations, version information, classification (IRDI dictionary entries below `Server/Dictionaries`) and structure, stores, conformance evaluation, hosting. Log messages use the EventId block 8300 to 8379, after Pumps (8100) and Scales (8200). 3. **`feat(amb)`: client package** (`src/Opc.Ua.AMB.Client/`, about 3.0k lines). Calls AddLink and RemoveLink through the generated `DocumentationLinksTypeClient`. 4. **`test(amb)`: test suite** (`tests/Opc.Ua.AMB.Tests/`, 163 tests, about 8.8k lines; commit 6 adds one more). 5. **`docs(amb)`: guide and samples**: `docs/AMB.md`, links from `README.md`, `docs/README.md`, `docs/Samples.md` and `docs/AliasNames.md`; `samples/AMB/AmbServer` (AMB next to `AddMachinery`: two DI devices and a Machinery press as assets, with a simulation) and `samples/AMB/AmbClient`. The sample server listens on 62553, which no other sample uses. 6. **`feat(amb)`: tell the links users added from the links of the manufacturer.** OPC 10000-110 gives a client no way to see which links `RemoveLink` accepts (§10.5.4: only those `AddLink` created). As §10.5.1 lets vendors add metadata Properties to the links, the server gives every link `AddLink` created, and every such link restored after a restart, the `Boolean` Property `UserLink` (`true`), qualified with its namespace of server-specific types, with a NodeId derived from the persisted link identifier; declared links carry none, and `RemoveLink` deletes it with the link. `DocumentationLinkRecord` gets the additive `IsUserLink`, which `ReadDocumentationLinksAsync` fills with one Browse over all links and one Read of the markers found (comparing the name only, as the namespace is configurable; `false` for a server that does not mark links). The name is shared as `DocumentationLinkProperties.UserLink`. `UA.slnx` and `.azurepipelines/expected-packages.txt` get the new projects and packages in the commit that adds them. All projects target `net48;net8.0;net9.0;net10.0` through the repository properties (`LibTargetFrameworks`, `TestsTargetFrameworks`, `AppTargetFrameworks`); `IsAotCompatible` and `PublishAot` are set from net10.0 on, as in the other companion projects. ## Tests - **`Opc.Ua.AMB.Tests`: 164 tests**, most of them against a hosted server over real client sessions: - the model: the SHA-256 of the NodeSet and the generator behaviours AMB is the first companion model to rely on; - each building block in-process and over a session: discovery through every category with NodeVersion and model change events, AssetId writes, health alarms and maintenance activities through all transitions with acknowledgement, AddLink and RemoveLink, locations and structure, the advertised conformance units and the facet; - a restart with the file system stores (links and AssetId survive); - the `UserLink` marker: present on an added link only, the same NodeId after a restart, removed with `RemoveLink`, and `IsUserLink` on the client in the snapshot, after `AddLink`, after a restart and after `RemoveLink`; - composition with `AddOpcUaDi` (both startup orders), `AddPumps` and `AddMachinery`; - `AmbClient` end to end against a hosted server, including a Machinery machine as an asset. - **Results (macOS, local):** 164 passed / 0 failed on net10.0 and on net8.0. .NET Framework is covered by the fork CI's Windows legs. - **Coverage** of the three AMB assemblies (coverlet with `tests/coverlet.runsettings.xml`, net10.0, generated `*.g.cs`, `*TypeClient` proxies and `*Log` classes excluded): lines 93.9 % (4890 of 5206), branches 84.0 % (1294 of 1539); model 100 %, client 94.5 %, server 93.4 %. - **Build:** the three packages, the tests and the two samples build without warnings for net48, net8.0, net9.0 and net10.0; `UA.slnx` builds in Release (its warnings are all in files this PR does not touch, as on master). `dotnet format` (whitespace, style, analyzers with `--severity info`) reports nothing on the 95 changed C# files. - **Samples:** `AmbServer` and `AmbClient` ran against each other on net10.0: the client discovered the three assets, printed their snapshots and the location hierarchies, added and removed a documentation link and streamed alarms and maintenance. - Fork CI (full matrix incl. macOS) and CodeQL: green on the same five package commits before the rebase onto the current #4650 head — [CI run 37144486107](https://github.com/biancode/UA-.NETStandard/actions/runs/37144486107), [CodeQL run 37144487873](https://github.com/biancode/UA-.NETStandard/actions/runs/37144487873). The run on the current head (rebased, plus the user-link commit; AMB tests 164/164 locally on net10.0) is in progress: [CI run 37203517457](https://github.com/biancode/UA-.NETStandard/actions/runs/37203517457). ## Notes for review - **Size.** About 26.8k lines, larger than each of the companion PRs #4577 to #4580. Without changes to the code it splits into the model, server and server tests, and the client, end-to-end tests, samples and docs on top; the issue asks which you prefer. - **Interfaces on types.** §9.4.1 and §12.1 ask for alarm and condition types that implement `IRootCauseIndicationType` and `IMaintenanceEventType`. The server creates subtypes of the DI alarm types in its own type namespace (`urn:opcua-netstandard:amb:types`); `UseServerDefinedAlarmTypes = false` uses `HasInterface` on the instance instead, and then does not report the units that need the type-level interfaces. - **Conformance unit names** follow Tables 54 to 56; the profile database has no AMB entries yet. The QualifiedNames are in namespace 0, as DI and Machinery publish theirs (question 5 in the issue). - **Generator gap.** Children a NodeSet gives to a top-level instance (here the `NodeVersion` properties of the alias categories) get no identifier constants. The fix changes the identifiers of other generated models, the reference server's among them, so it is not part of this PR; `AmbModelTests.NodeVersionHasNoIdentifierConstant` records it and fails once the generator emits the constants. - **net48.** The code that the SDK so far only built for .NET 8 and later uses the `NET5_0_OR_GREATER` pattern of `JsonEncoder` for `Enum.IsDefined` and API forms that .NET Framework has (`string.Format`, `Guid.ToString(format, provider)`); the tests use a small async file helper instead of `File.WriteAllTextAsync`. - **User links.** The `UserLink` Property is metadata of this server, not an AMB name: commit 6 explains why and how a client uses it. Changed lines of that commit covered: 69/70 (the one line left is the loop continuation for a link that has other Properties but no marker). - **Known limitations** are listed in the guide: the `DeviceHealth` SourceTimestamp across a restart, removing optional maintenance members once set, `PatchIdentifiers`, locations only at the deepest level, and the history units. ## Related Issues - Part of #4649 - Stacked on #4650 ## 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. (The AMB suite on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
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> |
||
|
|
01d6b5d26c |
Machinery: the OPC 40001 companion SDK with Jobs, Result Transfer, Process Values and Energy (#4578)
> Tracking issue #4576. #4577 is merged, so this branch is rebased onto > `master` and contains only the Machinery commits. ## Summary Second of four stacked PRs. Adds the OPC UA for Machinery series as model packages plus a server and a client SDK, following the companion pattern of DI and Robotics, and the two models it depends on that the repository does not carry yet. | Package | Content | |---|---| | `Opc.Ua.IA` | OPC 10000-200 IA 1.01.4, **moved out of `Opc.Ua.Robotics`** (see below) | | `Opc.Ua.ECM` | OPC 34100 Energy Consumption Management 1.0.1 | | `Opc.Ua.PADIM` | OPC 30081 PA-DIM 1.02.0 with the IRDI dictionary | | `Opc.Ua.Machinery` | OPC 40001-1 Basic Building Blocks 1.04.1 | | `Opc.Ua.Machinery.ProcessValues` | OPC 40001-2 Process Values 1.00.0 | | `Opc.Ua.Machinery.Jobs` | OPC 40001-3 Job Management 1.0.1 | | `Opc.Ua.Machinery.Energy` | OPC 40001-4 Energy 1.00 | | `Opc.Ua.Machinery.Result` | OPC 40001-101 Result Transfer 1.01.0 | | `Opc.Ua.Machinery.Server` | `AddMachinery()` / `ConfigureMachinery(...)`, fluent machine builder, server-driven state machines, process values, job management over the shared ISA-95 V2 binder, energy, result management incl. `GenerateFileForRead` | | `Opc.Ua.Machinery.Client` | machine discovery, identification, state observation, process values, job and result access | OPC 40001-101 `ResultTransferType` redeclares `GenerateFileForRead` with its own arguments; the generator fix for such overrides, merged with #4577, is what makes it appear on the `ResultTransfer` instances. All NodeSets are byte-identical to `UA-Nodeset/latest`. Each model carries a pinned `Model/*.NodeIds.csv` the generator validates, next to the unmodified upstream table; `tools/nodesets/derive-identifier-table.py` derives the former from the latter. ## Changes to existing code - **IA moves out of Robotics.** Machinery needs IA as well (OPC 40001-1 types `Stacklight` with `BasicStacklightType`), and two assemblies generating the same model under the same prefix collide at any consumer referencing both. The NodeSet moves verbatim, so no generated type changes; the prefix stays `Opc.Ua.IA`. Source-compatible, but IA types now live in `Opc.Ua.IA.dll` rather than `Opc.Ua.Robotics.dll` — binary-breaking for consumers of the Robotics package. - **`samples/OpenUsd/GeneratorServer`** drops its hand-reduced Machinery NodeSet copy and the script that produced it (a workaround for the generator defect fixed by #4577) and references `Opc.Ua.Machinery` instead. - `README.md` and `docs/README.md` list the new models; the README also corrects the Industrial Automation entry (Part 200, not OPC 40001-1). ## Review follow-ups Separate commits after the docs/samples commit, so the reviewed state stays comparable. Each fix comes with a test that fails without it. - Copilot review: a transition is committed only while the state machine is still in its source state; optional Jobs/Result namespaces the server does not load are reported as absent, as documented; resources of registered machines (result transfer, job-management change pumps) are released with the node manager (`DisposeMachineryResourcesAsync` for a custom DI manager); `NodeId`/`LocalizedText` use their null sentinels instead of `Nullable<T>`. - Own review with the rubric in `.github/copilot-instructions.md`: a failed browse call propagates from the client instead of reading as "no children"; the predefined result metadata compares `CreationTime` against the `DateTimeUtc` sentinels (the `DateTime` conversion shifted `DateTime.MaxValue` by the host's UTC offset); process values below `Monitoring` are found; a second metering point named `Main` is rejected; the zero-point event is reported when the handler throws; smaller state-sharing and documentation corrections. ## Testing - `tests/Opc.Ua.Machinery.Tests`: 162 tests on net8 and net10 and with the netstandard2.1 libraries; the integration tests connect a real client session over `opc.tcp` to a hosted server. - With IA extracted: Robotics 636, OpenUsd 921, DI 433 and SourceGeneration 263 tests pass. - `UA.slnx` builds for net472, net48, netstandard2.0/2.1 and net8/9/10 with SDK 10.0.401 and no warnings. - Changed-line coverage of this layer (`check-coverage.ps1`): 91.68 % (6059 of 6609 changed lines, against `master`). - Fork CI, full PR matrix incl. macOS, on the current master: green, all 184 jobs ([run 36582685653](https://github.com/biancode/UA-.NETStandard/actions/runs/36582685653)). CodeQL: green ([run 36582689589](https://github.com/biancode/UA-.NETStandard/actions/runs/36582689589)). - Beyond the in-repo tests the implementation has been validated end to end against a separate, non-public OPC UA client and server. ## Reviewing Commits are ordered so each builds: IA extraction → identifier-table tool → ECM/PA-DIM → Machinery models → server → client → tests → docs/samples → GeneratorServer sample → review follow-ups. The model commits are large only because of the NodeSets and identifier tables; the hand-written code starts at the server commit. --------- Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
ac8dc531ee |
Source generator, ISA-95 Job Control and DI changes for companion SDKs built on referenced models (#4577)
## Summary First of four stacked PRs that contribute complete implementations of the Machinery (OPC 40001-1 with Job Management, Result Transfer, Process Values and Energy), Pumps (OPC 40223) and Scales (OPC 40200, with PackML OPC 30050) companion specifications — tracked in #4576. It follows the split used for Robotics (#4123–#4127): this PR carries only the changes to **existing shared code** those SDKs need, and adds no project. Each change stands on its own and is covered by tests in this PR; the companion models are what surfaced them. Rebased onto master after #4569. The one conflict was in `NodeSetToModelDesign.cs`: #4569's shared `AssignSymbolicIds` already makes `Import` and `GetImportedSymbols` derive the same ids, and `NormalizeParentNodeId` moves into its pre-pass. The Copilot review is addressed in the last six commits (see "Review follow-up"). ## Changes ### Source generator (`tools/Opc.Ua.SourceGeneration.Core`) - **VariableType data type across assemblies.** A model that consumes a dependency through a referenced assembly's `[ModelDependency]` payload lost a VariableType's `DataType`/`ValueRank`, so `GetNodeStateClassName` dereferenced null and reported `MODELGEN003`. First hit: DI's `LifetimeVariableType` as used by Machinery. - **Method declaration ↔ method state identity.** A consumer re-declaring an inherited method referenced an `OwnerType_MethodMethodState` class the producer never generated. First hit: OPC 34100 ECM re-declaring the DI `LockingServices` methods. - **Browse names and reference types of inherited children.** An inherited child was emitted with `ReferenceTypeId = NodeId.Null` (invisible to a filtered Browse over the wire) and, where it lives in another namespace, with the declaring model's namespace. When the carried namespace is one the consuming model does not declare, the generated code names it by URI instead of emitting an empty `GetIndexOrAppend()`. - **Payload compatibility.** The three additions above travel in a length-prefixed, versioned declaration trailer after the existing trailers, so the node and child records keep their layout: an older reader ignores the capability bit and stops after the trailers it knows, and a payload without declarations is byte-identical to before. - **Symbolic ids of children listed before their parent.** Every `ParentNodeId` is normalized before any symbolic id is derived. No NodeSet in the repository has such an ordering today, so no generated symbol changes; OPC 30050 PackML (later in the stack) declares seven method `InputArguments` before their methods and failed with `MODELGEN025`. - **Methods overridden with another method-state class.** The subtype declares its own field and property for such a method, but `GetChildren` and `RemoveExplicitlyDefinedChild` skipped every overridden child and the supertype's field stays empty, so the method was missing from every instance (not browsed, no per-instance NodeId, not callable). The existing `MethodTypeOverride` test model has the gap; since #4555 generates a method with inline arguments as `MethodState`, OPC 40001-101 `ResultTransferType.GenerateFileForRead` hits it. It also affects models already in the repository: DI `SoftwareFolderType.Delete`, ISA-95 V2 `JobOrderReceiver.Update`, WoTCon `WoTDocument.Validate` and XRegistry `Group.Delete` / `Resource.Delete` now appear on their instances; the DI, ISA-95, WoTCon and XRegistry tests pass. ### ISA-95 Job Control (`src/Opc.Ua.ISA95.Server`) - **`Isa95JobControlV2Binder`** — the V2 receiver/response-provider binding moves out of `Isa95NodeManager` so OPC 40001-3 JobManagement can bind the same eleven verbs on its own instances instead of copying them. `Isa95NodeManager` behaviour is unchanged. - **`Isa95AddressSpaceOwnership`** — a second registration that would load the ISA-95 models fails immediately and names the current owner, as `DiAddressSpaceOwnership` already does for DI. - **`IIsa95JobResponseCatalog`** — a source for `JobOrderResponseList`, which the two state-filtered request methods cannot express. - **`IIsa95JobResponseChangeSource`** — a change stream of received responses, and of responses the provider drops after `ResponseRetention`; a response changes neither the order's state nor the order catalog, so the existing streams cannot report it. Both interfaces are implemented by `InMemoryIsa95JobControlProvider`, registered by `AddInMemoryIsa95JobControlProvider`, and count in `AddIsa95Server`'s provider detection and mixed-provider rejection. ### Device Integration (`src/Opc.Ua.Di.Server`, `src/Opc.Ua.Di.Client`) - **`IDiPostSetupContext.GetService<T>()`** — an optional lookup next to `GetRequiredService<T>()`, so a companion SDK can probe for a seam the server may not have filled without catching `InvalidOperationException`. The context still exposes no raw `IServiceProvider`. - **`DiNodeManager.ConformanceUnits` / `ServerProfiles` are virtual**, so a companion subclass appends the units its own model satisfies. - **DI client browse enumerators follow `BrowseNext`.** `DiTopologyClient.BrowseChildrenAsync` and `DiDeviceClient.BrowseFunctionalGroupsAsync` discarded the continuation point and returned an incomplete list from a paginating server. A bad status for the browsed node still means "no children" and a failed service call propagates; an outstanding continuation point is released whenever the enumeration stops early. ## Review follow-up | Copilot finding | Commit | |---|---| | Payload additions break older readers (both High) | `fix(sourcegen): move the dependency declarations into a trailer` | | Browse failures reported as empty, broad exception filter (Medium ×2); missing pagination tests (Low ×2) | `fix(di-client): let a failed browse call propagate` | | New provider facets missing from detection (Medium) | `fix(isa95): count the response facets as Job Control providers` | | Second review: continuation point kept after a failed BrowseNext | `fix(di-client): release the continuation point after a failed BrowseNext` | | Second review: purged responses not reported | `fix(isa95): report dropped job responses on the response change stream` | | Second review: missing V2 namespace cast to 65535 | `fix(isa95): reject a missing Job Control V2 namespace in the binder` | ## Points for the reviewer - `GetService<T>()` adds a member to a public 2.0 interface. The only implementation is the internal `DiPostSetupContext`; an external implementer would have to add it. - `DiDeviceClient.BrowseFunctionalGroupsAsync` used to throw for a bad status of the device node; like `DiTopologyClient` it now yields nothing. - The commits are self-contained and ordered; reviewing commit by commit is the easiest route. ## Testing - New and extended tests: `ModelDependencyV1Tests`, `NodeStateGeneratorTests`, `NodesetIdentifierSidecarTests`, `InMemoryIsa95JobControlProviderV2Tests`, `Isa95ServerBuilderTests`, `Isa95JobControlV2BinderTests`, `DiDeviceClientTests`, `DiTopologyClientTests`, `OpcUaServerDiBuilderExtensionsTests`. - `UA.slnx` builds for net472, net48, netstandard2.0/2.1 and net8/9/10 with SDK 10.0.401 and no warnings. - Tests on net10: SourceGeneration.Core 4266 (8 skipped), SourceGeneration 263, ISA95 144, DI 433; the first three also on net8. The changed projects build for net48 and netstandard2.0 without warnings. - Changed-line coverage (`check-coverage.ps1`): 91.1 % (594 of 652 lines). - The companion stack on top (#4578, #4579, #4580) is rebased onto this head. - Fork CI, full PR matrix incl. macOS, for this head (running): https://github.com/biancode/UA-.NETStandard/actions/runs/36558506011. CodeQL: https://github.com/biancode/UA-.NETStandard/actions/runs/36558509242. The previous head (before the second review round) passed every Windows and Linux job: https://github.com/biancode/UA-.NETStandard/actions/runs/36542929585. |
||
|
|
20c2d77d57 |
Fix Core/Types audit findings and conform them to the OPC UA spec (stacked on #4569) (#4573)
> **Stacked on #4569.** The base branch is `romanett/practical-murdock-fd9c56`, so this diff shows only the commits of this PR. Retarget to `master` after #4569 merges. ## Summary Bug audit of `Opc.Ua.Core`, `Opc.Ua.Core.Types`, `Opc.Ua.Types` and `Opc.Ua.Core.Schema`, with the findings fixed and the fixes grounded against the OPC UA specification (reference.opcfoundation.org). 133 commits in four groups: 1. **Audit fixes** (`Fix <ID>: ...`): 116 findings from a read-only audit, each with a regression test. IDs follow the audit scopes: T1-T6 `Opc.Ua.Types`, C1-C4 `Opc.Ua.Core`, S1 `Opc.Ua.Core.Types` + `Opc.Ua.Core.Schema`. 2. **Review fixes** (`Fix review <ID>: ...`): issues a code review of group 1 found, plus lower-severity leftovers. 3. **Spec fixes** (`Spec: #<n> ...`): the 18 places where an earlier fix only partly matched the spec text. 4. Test and analyzer follow-ups. Where `master` (#4516, #4522, #4540, #4564) or #4569 already fixed the same issue, their version was kept and this PR only adds what they didn't cover. ## Behaviour and user-facing changes ### Wire format and interoperability (breaking) - **PubSub AES-CTR (Aes128-CTR / Aes256-CTR)** now follows Part 14 Tables 156/157: - The counter block is `KeyNonce[4] | MessageNonce[8] | BlockCounter[4]`, and the block counter starts at 1. - The SecurityHeader nonce is 8 bytes: `Random[4] | UInt32 SequenceNumber`, with the sequence number starting at 1. - SKS key data is split with a 4-byte KeyNonce, giving 52 / 68 bytes. - **Not interoperable with earlier releases of this stack**, but matches the spec. There is a test against the independent RFC 3686 vector. - **User token signature** (Part 4 6.1.8): - The signed data is now built from the user token policy's SecurityPolicy on both client and server. - It also supports the SecurityMode None layout `ServerNonce | HASH(ServerCertificate) | ClientNonce`. - New overload `SecurityPolicyInfo.GetUserTokenSignatureData(..., MessageSecurityMode)`. - **Secure channel:** - The ECC AEAD `LastSequenceNumber` is correct across the 32-bit sequence wrap (Part 6 6.8.1). - The incoming chunk counts against MaxChunkCount/MaxMessageSize. - The sender no longer lets a message through that is 24 bytes over MaxMessageSize. - The OPN response signature is verified without overrunning the buffer (RSA_DH with differing key sizes). ### Sessions (server) - **CreateSession nonce:** - The client nonce is kept on None channels, because the None-mode user token signature covers it. - A nonce over 128 bytes returns `Bad_NonceInvalid` on every channel (Part 4 5.7.2.3). - The duplicate-nonce check runs only on secured channels. - A non-empty nonce under 32 bytes is still accepted on None, for compatibility. - **ActivateSession:** an embedded `EccEncryptedSecret` signing certificate is validated with the server's peer certificate validator (Part 6 6.8.3). Byte equality with the client certificate is only the fallback when no validator is available. New opt-in interface `ICertificateValidatorProvider`. - **FindServers / GetEndpoints:** `GatewayServerUri` and `DiscoveryProfileUri` are no longer swapped. ### Write, Read and Call (NodeState / node managers) - **UserWriteMask:** - Enforced for non-value attributes and for VariableType Value writes (`ValueForVariableType`), returning `Bad_UserAccessDenied`. - An unconfigured UserWriteMask (0, no handler) is not enforced, and reads back as the WriteMask so Read and Write agree (Part 3 8.60). - A throwing or Bad `OnReadUserWriteMask` handler fails only that item. - **Index-range writes:** - Slices are merged into the cached value under the attribute lock, also for `OnWriteValueAsync` handlers. - A data-change notification is still produced when the cache cannot absorb the slice. - The slice must match the unclipped range, otherwise `Bad_IndexRangeDataMismatch` (new status code in `Opc.Ua.Types`). - A range past the target returns `Bad_IndexRangeNoData`. - A handler without index-range support returns `Bad_WriteNotSupported` (Part 4 7.27, 5.11.4.4). - **Index-range reads on matrices** return partial results clipped to the source, instead of allocating the requested size (a Read with `"0:40000,0:40000"` no longer allocates about 1.6e9 elements). - **DataTypeDefinition writes:** - A value that isn't a decoded DataTypeDefinition, or whose kind doesn't match the DataType category, returns `Bad_TypeMismatch`. - Null is rejected for concrete Structure/Enumeration/OptionSet types. - WriteMask is checked first (Part 3 5.8.3). - **AnalogItem:** a null write skips the InstrumentRange check again. - **Method call:** the async fallback passes the ObjectId to the method handlers. - **Redundancy:** `DistributedValueParticipation` merges index-range writes and applies the index range on reads. - **Change masks:** `ClearChangeMasks` takes the bits atomically, so concurrent changes are no longer lost. - **`AccessLevelEx`:** preserved by Clone/Export. ### Alarms & Conditions (Part 9) - **Acknowledge / Confirm / AddComment:** - Accept any EventId the condition or its branches reported while that state is still pending (5.7.3). - An unknown id returns `Bad_EventIdUnknown`. - An already handled state returns `Bad_ConditionBranchAlreadyAcked` / `...AlreadyConfirmed`. - AddComment on a branch no longer changes the trunk, and the branch table is re-keyed when a branch's EventId changes. - **Shelving:** - OneShotShelve without MaxTimeShelved no longer overflows the timer. - A timed shelve is measured on the monotonic clock, so wall-clock steps don't delay the unshelve. - The state is named `OneShotShelved`. - **Audit events:** - They report failure for rejected method calls. - `SelectedResponse` is a UInt32. - A rejected negative Respond leaves SelectedResponse unset. - **`ContentFilterElement.Validate`:** an unknown FilterOperator is reported per element as `Bad_FilterOperatorInvalid`, instead of failing the whole service call. ### ContentFilter evaluation (Part 4 7.7.3, on top of #4564) - **Bitwise operators:** - The result takes the size of the larger operand. - Boolean, numeric String and one-element array operands are converted to integers implicitly. - Types without an implicit integer conversion give NULL. - **Strings:** ordering is ordinal, no longer culture dependent. - **Cast:** restricted to the Table 121 conversions, and QualifiedName to LocalizedText is added. The general `Variant.ConvertTo` keeps its wider set. - **Null and empty operands:** - `Like` with a NULL operand is NULL. - Empty arrays of the same DataType compare equal. - Null-string operands are NULL everywhere. ### Encoders and built-in types - **XML:** - Strings keep leading, trailing and whitespace-only content (Part 6 5.3.1.5). NodeSet import opts back into treating whitespace-only strings as empty with the new `XmlDecoder.TreatWhitespaceOnlyStringsAsEmpty`. - Layout whitespace around `<Identifier>` values is ignored, and trailing control characters are stripped from `s=` ids. - `XmlElement` ignores DTDs without expanding entities. - **JSON:** - A DateTime without an offset is read as UTC. - An unmappable `nsu=` NodeId and an unknown `svu=` ExpandedNodeId decode to a namespace-0 String NodeId (5.4.2.10 / 5.4.2.11). - `svr=` is rejected in a NodeId field. - Bad matrix dimensions return `BadDecodingError`. - The non-strict body stack stays balanced after an error. - The nesting limits match between encoder and decoder. - **Binary:** - `MaxByteStringLength` no longer applies to strings. - ExtensionObject bodies count toward the nesting limit on seekable streams (and in the XML encoder). - **Parsing:** - The NodeId parser requires `i=`/`s=`/`g=`/`b=`. - `ExpandedNodeId.Parse` rejects an empty `nsu=`/`svu=`. - NumericRange rejects a trailing `,` with `Bad_IndexRangeInvalid`. - RelativePath escapes `#`. - **Status codes:** `StatusCode.WithSubCode`/`WithCodeBits` keep the severity bits and look up the symbolic id for the new code. - **Variant / VariantHelper:** - `TryCastTo` returns false on a type mismatch. So `DataValue.GetValue<T>(default)` returns the default, and `GetValueOrDefault` / `CastTo` throw `BadTypeMismatch` as documented. - `System.DateTime` casts are supported. - Concrete encodeable arrays (e.g. `Argument[]`) no longer throw. - `ConvertTo` keeps one-element and empty arrays as arrays and checks the conversion is legal for empty arrays. - `CastFrom(object[])`, `CastFrom(Enum[])` and the obsolete `Variant(object)` constructors keep the array shape (previously a one-element array collapsed to a scalar and an empty one to Null). - `TypeInfo.Unknown` / `Variant.Null.TypeInfo` report `ValueRanks.Any`. - **Equality and hashing contracts fixed:** - Types: Matrix/MatrixOf, typed-null Variants, NodeId (the namespace index is now hashed), ExpandedNodeId, LocalizedText, DiagnosticInfo, StructureDefinition/EnumDefinition, CertificateIdentifier, UserIdentity and DateTimeUtc. - `MatrixOf.Dimensions` returns a copy. - Decimal equality and hashing now run in bounded time. - **Localized text:** a LocalizedText with locale `"mul"` and no usable translations, or null client locale ids, no longer throw. ### NodeSet, address space and utilities - **NodeSet export/import:** - Server index 0 is the local server on import and export, so local ids are no longer written as `svr=65535`. - `StructureDefinition.BaseDataType` is filled from the supertype on import. - **`NodeTable.Import`:** replaces duplicates, keeping the references neighbouring nodes hold. `TypeTable.Add` detaches a re-parented type from its old supertype, and `Node.Clone` gets its own ReferenceTable. - **`StringTable` / `NamespaceTable`:** capped at UInt16 indexes (`BadEncodingLimitsExceeded`) for Append and Update, instead of silently wrapping. - **Small fixes:** - `UnsecureRandom.Shared` is seeded randomly. - The peer's RetryAfterMs is clamped to one day. - The `CancelAsync` and `string.GetHashCode(StringComparison)` polyfills match BCL behaviour. - `CoreUtils.IsEqual` compares strings ordinally. ### Schema generation and ComplexTypeSystem - **JSON schema:** describes the 1.05 layout with the compact/verbose null rules, and now validates real `JsonEncoder` output. - **XSD:** - `EncodingMask` (`xs:unsignedLong`) only for StructureWithOptionalFields. - Guid and StatusCode are complex types. - Standard types are in the `Types.xsd` namespace. - **Structure-backed OptionSets** are described as `{Value, ValidBits}`. New `UaTypeDescription.IsStructureOptionSet` property and constructor overload. - **ComplexTypeSystem:** - A malformed type in a server dictionary is skipped per type instead of discarding the whole dictionary. - Cyclic supertype walks are bounded. - Declared definitions are no longer changed. - A dictionary validator is kept only if the dictionary parsed. ### Certificates and trust lists (Part 4 6.1.3, Part 12) - **Revocation:** - Checked for issuers supplied in the peer's chain and for inline-trusted issuers, against the CRLs in the trusted and issuer stores. - A store without CRL support no longer hides "revocation unknown". - `RejectUnknownRevocationStatus` is honoured for inline-trusted issuers unless the entry sets `SuppressRevocationStatusUnknown`. This changes the strict case of `ExplicitIssuerChainIsHonoredWithoutStorePathAsync`. - **Basic constraints / path length** violations fail with `Bad_CertificateInvalid`. - **`TreatAsInvalid`:** - Now yields `Bad_CertificateUntrusted`, which can't be suppressed or auto-accepted. - It is honoured on individual trust-list entries even when the certificate is also in a store. - **`EccEncryptedSecret`:** enforces the result of validating the sender certificate. - **`TrustListTransaction`:** - Routes each CRL to the store holding its issuer (TrustedCrls/IssuerCrls). - Validates staged CRLs, including the store's CRL support, before changing anything. - Applies certificates before CRLs. - **Other:** - One failing `CertificateChanges` observer no longer stops the others. - `InMemory:` stores share their contents per path, and `InMemoryStoreProvider` is `IDisposable`. ### WoT (`Opc.Ua.Types/Wot`) - **Projection security:** projection security schemes whose qualified names collide are refused. New diagnostic `WotDiagnosticCode.ProjectionSecurityConflict = 6063`. - **Input handling:** - Invalid Unicode (lone surrogates, bad UTF-8) is rejected as `JsonException`. - External-schema results are keyed per affordance. - A duplicate security-floor member is rejected. - `nsu=` ids are percent-encoded (Part 6 5.1.12). - The configured JSON depth is honoured on re-parse. - An oversized document is reported before it is parsed back. - **Performance and limits:** - Subtype-graph validation, `componentOf` linking and the namespace table are now linear. - Namespaces are capped at UInt16. ### New or changed public API - `ICertificateValidatorProvider` (server) - `SecurityPolicyInfo.GetUserTokenSignatureData(..., MessageSecurityMode)` overload - `XmlDecoder.TreatWhitespaceOnlyStringsAsEmpty` - `UaTypeDescription.IsStructureOptionSet` plus a constructor overload - `WotDiagnosticCode.ProjectionSecurityConflict` - `StatusCodes.BadIndexRangeDataMismatch` in the `Opc.Ua.Types` list - `InMemoryStoreProvider : IDisposable` ## Not changed / open - **Deferred:** - S1-14 (BSD OptionSet size needs the base type in `UaTypeDescription`, and BSD is deprecated). - C2-7 (restoring the own-certificate trust exemption would accept certificates the spec says to reject; removing the dead code is a separate decision). - A1-4 (config values keep surrounding whitespace, as the pre-April-2026 loader did). - H-2 (JsonDecoder has no real nesting counter). - A2-3 (the StringTable cap also applies to diagnostics tables; default `MaxArrayLength` already bounds them). - **Needs a product decision:** - The general `ConvertToString` writes a QualifiedName as `ns:Name`, while Table 121 drops the namespace. - Short (< 32 byte) client nonces are accepted on None channels. ## Verification - Every commit added a regression test that fails without its fix, where practical. - Before the rebases, with all audit and review fixes (before the `Spec: #…` commits): 0 build warnings and 16 test suites green on net10.0. The suites: Types, Core, Core.Encoders, Core.Security, Core.Schema, Client, Client.ComplexTypes, Server, Sessions, Gds, History, Features, Redundancy.Server, PubSub, WotCon and WotCon.Bindings. Core hit its known intermittent end-of-run hang once, and reruns were clean. - The `Spec: #…` fixes were verified per group on their own suites. Those include PubSub.Udp, Redundancy.PubSub, PubSub.Server, Configuration and SourceGeneration.Core. - **Not yet built or tested on this base.** It was rebased onto master (resolving overlaps with #4522, #4540 and #4564) and then onto #4569. This PR stays a draft until the full run on this base is green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
442889cba5 |
Fix source generator audit findings; conform structure-field matrices and type metadata to OPC UA 1.05 (#4569)
# Description
This PR fixes a bug audit of the source generation projects
(`tools/Opc.Ua.SourceGeneration*`: model loading, NodeSet import, all
emitters, the Roslyn hosts and packaging). It also fixes a max-effort
code review of those fixes, and a check of every spec-relevant change
against the OPC UA 1.05 text.
- **Scope:** about 85 audit findings, 48 review findings and 32
spec-grounding items were addressed. Each fix has a regression test that
fails without it.
- **Wire change:** it includes one deliberate, stack-wide wire change:
**structure-field matrices now follow OPC 10000-6 5.2.5 Table 28**.
Most changes are internal correctness fixes. The sections below list
every change a user can observe: wire formats, generated code and API,
generated address-space content, diagnostics and runtime behaviour.
## Behaviour / user-facing changes
### Wire format (encoders)
- **Binary inline matrices** (OPC 10000-6 5.2.5 Table 28).
- **Layout:** a multi-dimensional *structure field* is encoded as
`Int32[] Dimensions`, then the values **without a length prefix**, with
at least 2 dimensions.
- **Empty and null:** any dimension <= 0 means no values follow, and a
null matrix is `-1`. An empty matrix is written as `[0,0]`.
- **Compatibility:** this is **not compatible with earlier builds of
this SDK** for matrix fields. It applies to generated types,
`[DataType]` types, `Structure`, client ComplexTypes and
`WriteEncodeableMatrix`.
- **Unchanged:** Variant matrices (5.2.2.16) keep their old encoding.
- **Declared rank decides matrix vs array.** A 1-D `MatrixOf<T>` in an
array field or a Variable value is written as an array again.
Matrix-declared fields go through the new public
`EncoderExtensions.WriteInlineMatrixValue`, which generated code also
uses. A non-empty 1-D value in a matrix field throws `BadEncodingError`.
- **Decoder hardening:**
- Before allocating, the decoder bounds all matrix dimensions against
overflow, `MaxArrayLength` and the remaining message length.
- The decoder rejects:
- a matrix with fewer than 2 dimensions;
- a populated matrix whose rank differs from the field's declared rank;
- a union SwitchField greater than the number of fields;
- an EncodingMask with unassigned bits set.
- **Encoder `MaxArrayLength`:** the encoder enforces `MaxArrayLength`
for inline matrices, as it already did for arrays.
- **XML structure fields** (5.3.1, 5.3.4, 5.3.5).
- **Fields:** `Structure` and client ComplexTypes write fields like
typed fields (`<A>1</A>`, array elements directly) instead of a Variant
body.
- **Matrix fields:** `Dimensions` and `Elements` go directly inside the
field element, with no `<Matrix>` wrapper.
- **Empty matrices:** written as nil, because XML dimensions must be
greater than 0.
- **Compatibility:** decoders still accept the old shapes.
- **Enum fixes:** enum array elements with value 0 are no longer
dropped, and enum array element names are fixed.
- **Other encoding fixes:**
- A null `DiagnosticInfo` structure field writes `0x00` instead of
nothing.
- A null scalar field in a runtime `Structure` is written as its default
instead of zero bytes, which used to shift every following field.
- A Variant holding an empty matrix encodes as an empty array instead of
throwing.
- `Variant.Copy` keeps the matrix identity of null and empty matrices.
- **NodeSet export and the DataContract surrogate** write Variable
values with the Variant rules again. A null or empty `MatrixOf` variable
no longer produces invalid XML or unreadable NodeSets.
- **`JsonEncoder.CanOmitFields`** is `true` only for Compact encoding.
Verbose and RawData JSON include default-valued fields.
### `[DataType]` source-generated types
- **Default-value handling** (OPC 10000-6 5.4.1, Table 1).
- **Encode:** a field is omitted only when its value *and* its declared
default are the type's default (0, false, null). Fields with a
non-default initializer or constructor default, and empty strings, are
always written, so configuration files get slightly larger.
- **JSON decode:** a field missing from JSON decodes to the type
default.
- **XML decode:** a field missing from XML keeps its declared default,
which older configuration files rely on.
- **Why:** this fixes silent round-trip loss, for example
`SamplingInterval = 0` read back as `-1`.
- **Nested `[DataType]` types:** they are now generated inside their
containing types. Without an explicit id, their default TypeId and
XmlName are qualified (e.g. `s=Outer.Foo`); **wire identity change** for
nested types. Colliding activator names get a `_2` suffix.
- **Activators:** they are `internal` when the type, or any containing
type, is not public (was CS0060).
- **Matrix properties:**
- `MatrixOf<T>` properties use the inline matrix encoding.
- Matrices of non-sealed structures, or with
`StructureHandling.ExtensionObject`, are written as ExtensionObjects.
- Their published ValueRank is 2, because the rank can't be known from
`MatrixOf<T>`.
- **Published StructureDefinitions:**
- They include base-class fields.
- Nullable properties are no longer marked optional.
- Inline structure fields name their concrete type.
- **`[Flags]` enums:** they are published as plain enumerations with
their member values, which matches how they are encoded.
- **Other generated code:**
- `Clone` deep-copies arrays and matrices of structures, Variants,
ExtensionObjects and DataValues.
- `IsEqual` and `GetHashCode` are consistent: EncodingMask, active union
member and NaN handling agree.
- Pooled `Reuse()` restores the freshly constructed state through the
new `protected virtual ResetForReuse()`, and returns an instance to its
runtime type's pool.
- **Enum capture:** enum member values are captured culture-invariantly.
Negative values were lost on cultures that use U+2212 as the minus sign.
### Model-driven generator: generated code, API and file names
- **Removed API:** generated `*_Placeholder` proxy accessors (e.g.
`PowerTrainTypeClient.GetMotorIdentifier_PlaceholderAsync`) and fluent
`X_Placeholder()` accessors are removed; they could never resolve
anything.
- Proxy accessors now look children up by BrowseName with the right
namespace.
- `RoboticsClient` finds motors and gears by type and drives via
`IsDrivenBy`. Its topology snapshots used to be empty.
- **Renamed files:** generated files for a bound `[NodeManager]` are now
named `{Namespace}.{Class}.NodeManager.g.cs` (also `.Factory` and
`.FluentBuilders`).
- **Generated NodeState code:**
- Attributes are set through `baseState`/`nodeState` locals, so children
named like base attributes (e.g. `DataType`, `EventNotifier`) compile.
- Children that hide base members are declared `public new`, using names
reflected from the runtime.
- `global::` qualification is used throughout.
- **Proxies and fluent builders:** a proxy whose supertype is excluded
(e.g. Draft) derives from the nearest emitted ancestor. Fluent wrappers
and NodeState factories fall back the same way; they used to fail with
CS0246.
- **Derived structures:**
- The optional-field `EncodingMask` continues across inheritance.
- A derived structure over a base type from another model no longer
relies on `base.ResetForReuse`.
- **Default values:**
- NodeId, QualifiedName and ExpandedNodeId defaults resolve by namespace
URI (`context.NamespaceUris.GetIndexOrAppend`) and server URI.
- StatusCode, negative enum, `double`/`float` (shortest round-trip
literal) and escaped string defaults now compile.
- **Event records:**
- Vendor event records have `Decoder.GetStandardFields(NamespaceTable)`,
and their browse paths use the model's namespace.
- A field whose namespace is unknown to the session no longer matches a
namespace-0 field.
- Companion `StandardFields` is `[Obsolete]`.
- Vendor simple subtypes such as `Percent : Double` map to their
built-in type.
- **Other fixes:**
- `MethodState.CallAsync` passes the real `objectId` to synchronous
`OnCallMethod2` handlers; it used to pass `NodeId.Null`.
- Enumeration fields with an explicit `Identifier` keep it, instead of
taking a trailing `_<digits>`.
- Explicit NumericIds are no longer reused by automatic numbering.
- Dependencies are loaded in topological order.
### Generated address-space content
- **Arguments:**
- Methods without arguments no longer get empty
`InputArguments`/`OutputArguments` properties (OPC 10000-3 5.7).
- About 2,660 automatically assigned namespace-0 NodeIds of 15000 and
above shift. Every NodeId in `StandardTypes.csv` is unchanged.
- **Permissions:** type nodes and method declarations on types now carry
the model's `RolePermissions`/`AccessRestrictions`.
- **Restored nodes:** NodeSet import keeps nodes that were silently
dropped before:
- `NamespaceMetadata` objects and the NodeSet's own TypeDictionary
variables;
- ISA95 placeholders linked only non-hierarchically;
- instances whose parent is in another namespace.
- **Import details:**
- ValueRank > 1 without ArrayDimensions keeps its rank.
- `svr=` indexes map correctly.
- Invalid MinimumSamplingInterval values import as -1, not 0.
- Localized texts prefer the invariant or English translation.
- **StructureDefinitions:**
- They include inherited fields, including for subtypes without their
own fields (e.g. `AnonymousIdentityToken` now publishes PolicyId).
- The StructureType kind covers inherited AllowSubTypes fields.
- Unions ignore IsOptional.
- StructureFields never publish ValueRank 0; a OneOrMoreDimensions field
without dimensions becomes rank 2.
- Fields with ValueRank -2/-3 are published as scalar BaseDataType,
which is what is on the wire.
- **OptionSets:**
- Implicit fields get the next free bit.
- `OptionSetValues` uses null for undefined bits instead of "Reserved",
and structure OptionSets publish all their bits (no 32/96-bit cap).
- Numeric OptionSets are limited to their base type's width, and UInt64
bit 63 works.
- **Schemas:**
- Structures with inline-matrix fields are left out of the `.bsd`
(5.2.5).
- In the BSD, Variant-encoded and non-structure subtyped fields are
described as `ua:Variant`.
- In the `.xsd`, matrix fields are typed `ua:Matrix` with
`Dimensions`/`Elements`, and `EncodingMask` is required.
### NodeSet / model input handling
- **Model selection:**
- The newest NodeSet of a model is chosen by `ModelVersion` (SemVer 2.0
precedence), then PublicationDate. The `Version` attribute, which Annex
F says is not for comparison, is only a tie-break.
- The order is total and does not depend on input order; referenced
assemblies use the same rule.
- **Default names:**
- Invalid default Name/Prefix values are repaired (`201301ISA95` becomes
`_201301ISA95`, `class` becomes `class_`).
- Valid names are unchanged.
- Sanitising no longer depends on the OS.
- **`ModelUri` metadata** that doesn't match the NodeSet raises a
warning instead of silently losing the model.
- **Input detection:**
- Only XML files whose root is `ModelDesign` or `UANodeSet` are treated
as models.
- A malformed NodeSet stays a NodeSet input, so it no longer breaks the
ModelDesign pass.
- **`IdentifierFile` paths:** they are normalized (`./`, `../`), and the
suffix match requires a folder boundary.
- **Dependency payloads** (`ModelDependencyV1`) carry field IsOptional,
AllowSubTypes, ArrayDimensions and the producer's namespace tables in
skippable trailers, so old and new readers stay compatible.
### Roslyn host, diagnostics, packaging
- **New diagnostics:**
- MODELGEN036: `[NodeManager]` target not supported (nested, generic,
file-local or global namespace).
- MODELGEN037: `[DataType]` target not supported (struct, generic,
abstract, file-local or global namespace).
- MODELGEN038: the definition of an encodeable base type can't be found.
- **New validator errors:**
- a structure with both optional and subtyped fields (OPC 10000-6 F.13);
- multi-bit or duplicate OptionSet masks.
- **Diagnostic locations:** `[DataType]`/`[NodeManager]` diagnostics
point at real source locations again, so `#pragma` and `.editorconfig`
apply. MODELGEN010 is the exception.
- **Incremental caching:** pipeline values compare by value, so
unrelated edits no longer regenerate the whole model or all
`[DataType]`s.
- **`ModelSourceGeneratorIgnore`** on a ModelDesign input is honoured;
the design still resolves as a dependency.
- **Cancellation:** a cancelled generation run propagates
`OperationCanceledException` instead of reporting a MODELGEN003 error.
- **Stack package:** the `Stack` NuGet package now ships
`build/OPCFoundation.NetStandard.Opc.Ua.SourceGeneration.Stack.props`,
so `StackSourceGeneratorMode` and `StackSourceGeneratorExclude` reach
the compiler.
## Related Issues
- No tracking issue. The audit, review and spec-grounding reports are
available on request. Contains the matrix wire-format change discussed
for D-5 (OPC 10000-6 5.2.5 Table 28).
## Checklist
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
(`docs/SourceGeneratedDataTypes.md`, `docs/NodeManagers.md`, generator
readme)
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings. (`UA.slnx` 0 warnings / 0 errors on net48 and
net10.0)
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. The
affected suites were run on net10.0: SourceGeneration Core/Roslyn/Stack,
Types, Core.Encoders, Encoders.Fuzz, Client, Client.ComplexTypes,
PubSub, Core, Server, GDS, Configuration, Di, Robotics, ISA95,
Positioning, OpenUsd, XRegistry. net48 was built; its test legs, and the
post-rebase verification, run in CI.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
c4e0721a29 |
Add a documentation learning path and fix issues found while verifying it (#4555)
## Summary
### Documentation
- Stage the documentation index as a learning path: a first application,
independent client, server, and PubSub paths, then data and services,
companion models, production, advanced integration, and draft or
experimental capabilities.
- Add Getting started (repository samples or NuGet packages), OPC UA
concepts, Your first information model, and a production readiness
checklist.
- Add Native AOT application publishing, a first PubSub publisher and
subscriber exchange, and the UDP transport options.
- Add or expand contents navigation throughout `docs/`, and keep
contributor documentation separate.
- Correct outdated SDK examples and capability guidance, consolidate
repeated explanations into canonical guides, and repair local links and
heading structure.
- Rewrite What's New in 2.0 as a concise feature overview with short
linked details; keep migration instructions in the Migration Guide.
- Expand the feature catalogue with specification IDs, GDS, LDS, the OPC
10000-21 onboarding registrar, ISA-95, xRegistry, OpenUSD, and
sample-local models; distinguish draft status, explain crypto
pluggability, and bring performance guidance forward.
- Address review feedback on wording, active monitored items during
shadow reload, and the `Samples.md` filename and references.
- Regenerate the `opcua-v20-migration` plugin's bundled copy of the
migration guides so it matches the edited `docs/migrate/2.0.x` guides.
- Merge the latest `master`, preserving upstream resource-isolation
guidance within the task-based documentation index.
- Retain deferred Lingity findings where appropriate; this PR does not
claim that every document meets the Lingity score thresholds.
### Fixes found while verifying the guides
Each fix has regression tests that fail without it.
- **Client:** V2 subscriptions compared their lifetime with the
operation timeout instead of the session timeout, and logged a spurious
warning. `ISubscriptionEngineContext`, which is new in 2.0, gains a
`SessionTimeout` property; implementations must add it.
- **PubSub:** a subscriber now registers the `DataSetMetaData`
configured on its readers (OPC 10000-14 6.2.9.4), and the UADP decoder
decodes DataSetMessages that carry no ConfigurationVersion. Before, a
secured subscriber of the reference sample received no DataSets.
- **Source generator:** a ModelDesign method that declares its arguments
inline, without a method type, produced code that did not compile; it
now uses `MethodState`. The generated code of existing models is
unchanged.
- **MinimalClient sample:** waits for its first data-change notification
before it disconnects.
### Known issues, not addressed here
- UDP multicast: the publisher does not send on the interface that the
subscriber joins, and `MulticastLoopback` defaults to false. The PubSub
guide uses unicast for the exchange on one computer.
- The UDP metadata announcement is sent unsecured on secured writer
groups; a secured subscriber drops it with a warning.
- A server does not start in globalization-invariant mode, because
`ResourceManager` creates `CultureInfo("en-US")`. The Native AOT guide
cautions against the mode.
## Validation
- `git diff --check` and
`./.azurepipelines/validate-migration-plugin.ps1` (15 bundled migration
docs validated, plugin manifests synchronized).
- Local Markdown links, anchors, file-link casing, and contents lists
checked across all 87 documents under `docs/`. The index links every
top-level guide and every target of the previous index.
- Both Getting started routes, Your first information model, the PubSub
exchange, and the Native AOT publish of the Getting started client and
server were run as documented. The NuGet route and the tutorial use the
public `2.0.0-preview.5` packages.
- The changed libraries build for all target frameworks in Release
without warnings.
- Tests on .NET 10: Opc.Ua.SourceGeneration.Tests 199 passed;
Opc.Ua.SourceGeneration.Core.Tests 3,977 passed, 8 skipped;
Opc.Ua.Server.Tests `Fluent` category 621 passed; Opc.Ua.Client.Tests
subscription-engine tests 70 passed; Opc.Ua.PubSub.Tests 1,374 passed, 2
skipped; PubSub fuzz replay 145 passed. The new and affected tests also
pass on .NET Framework 4.8.
- Earlier merge validation: 174 resource-isolation tests passed on .NET
10.
- The full solution suite was not run locally.
Review and validation artifacts are maintained outside the repository.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
||
|
|
9b8a3b1188 |
Preserve node initialization and copied child ownership (#4544)
# Description Preserve populated derived node children through optional initialization and give copied children independent ownership by their copied parent. - Track initialization from a source in `NodeState` and guard generated optional-default recreation without removing the optional initialization hook. - Reuse `CloneChild` for dynamic children, generated typed children, and method argument properties. Preserve the clone root's existing external parent. - Add cross-model initialization/copy regressions, method-argument and nested-child ownership tests, and inherited alarm-subtree checks. This PR contains only the six files for this fix group. Certificate-store, runtime-type-loader, publish-queue and transport-exception fixes are separate work. ## Related Issues Fixes #4525. Fixes #4526. ## Validation Fresh focused Release/net10.0 checks on the final base: | Fixture | Passed | Failed | Skipped | | --- | ---: | ---: | ---: | | CrossModelInstanceTypeDefinitionGeneratorTests | 9 | 0 | 0 | | Types NodeStateTests | 227 | 0 | 0 | | Core StateTypesTests | 13 | 0 | 0 | All three test commands exited 0. `git diff --check` and the changed-file diagnostic check are clean. Regression proof was also executed in the isolated worktree: with the three production changes reversed, six generator cases failed with the missing derived Temperature child while three controls passed; two copy cases failed parent-identity assertions while the external-parent/absent-arguments control passed. The production fix was restored and the focused checks are green. The retained red runs preceded the later upstream synchronization; the affected production paths were unchanged by that synchronization. Windows win-x64 NativeAOT publish and native execution also passed: main 178/178, Historian 2/2, MCP 3/3, no skips, every invocation exit 0. These native runs preceded the final CTT-configuration/notes-only fast-forward; this PR's six files were unchanged. Local validation is intentionally change-scoped for this work, with the full matrix and repository-wide checks left to PR CI. Repository guidance and test assertions/timeouts were not modified. Local commands used a NuGet v2 source override and disabled audit only on the command line; CI feed/audit policy is unchanged. Earlier broad attempts are not claimed as a completed local gate: an unmodified-base durable restart timeout was reproduced before synchronization (the later durable run passed), and the net48 runner stalled after recording passing client tests. A direct client retry subsequently exited 0, but the full net48 matrix was not completed. Those results are not substituted for the focused evidence above. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING doc. - [x] I have added regression tests and demonstrated failure without the production fix. - [x] The focused tests pass; no new compiler diagnostics were reported by those builds. - [ ] Coverage and the full net48/net10.0 matrix are verified by PR CI. - [ ] All CI, CodeQL, and review feedback is addressed. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
|
96c5ef1623 |
Run the fluent configure pipeline in FluentNodeManagerBase (#4537) (#4538)
## Proposed changes Closes #4537. The configure pipeline now lives in `FluentNodeManagerBase`, so a hand-written fluent node manager only overrides `ConfigureAsync` and no longer writes `CreateAddressSpaceAsync` itself. `FluentNodeManagerBase.CreateAddressSpaceAsync` now: 1. loads the predefined nodes (as `AsyncCustomNodeManager` did before), 2. creates a builder for `NamespaceIndex` with `CreateFluentBuilder`, 3. awaits `ConfigureAsync(builder, ct)`, 4. runs `RegisterAuthoredNodesAsync` and then `CompleteConfigureAsync`, 5. finishes with `SealConfigurationAsync`, which seals the builder, replays `NotifyNodeAdded`, then activates behaviors and simulations. ### Breaking change (2.0) Some managers need a different order, for example a post-setup runner before sealing, a builder for another namespace, or work after sealing. They must stop calling `base.CreateAddressSpaceAsync`, because that would now run a second, empty pipeline and seal the manager-owned registries too early. These managers now call `LoadPredefinedNodesAsync(SystemContext, externalReferences, ct)` and keep driving their own pipeline: - the source-generated `NodeManager` template (the ordering test in `NodeManagerGeneratorTests` is updated) - `DiNodeManager`, `Isa95NodeManager`, `PositioningNodeManager`, `RobotIntentNodeManager`, `VisionNodeManager` - `samples/OpenUsd/SiteCompositionServer/SiteNodeManager` These now use the base pipeline: - `samples/FluentApi/BasicFluentApiServer/BasicFluentNodeManager` becomes a single `ConfigureAsync` override. Before this change it never sealed its builder. - the hosting `FluentNodeManager` (from `AddNodeManager(namespaceUri, build)`) becomes a `ConfigureAsync` override. It now seals through `SealConfigurationAsync`, so `OnNodeAdded` handlers wired in the build delegate fire. Before, it called `builder.SealAsync()` directly and they did not. Docs (`NodeManagers.md`, `DeviceIntegration.md`) are updated. `NodeManagers.md` gets a short hand-written `ConfigureAsync` example. ## Types of changes - [x] Breaking change (fix or feature that would cause existing functionality to change) - [x] Enhancement (non-breaking change which adds functionality) ## Checklist - [x] I have read the CONTRIBUTING doc. - [x] I have signed the CLA. - [x] I ran tests locally with my changes, all passed. - [x] I fixed all failing tests in the CI pipelines. - [x] I fixed all introduced issues with CodeQL and LGTM. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added necessary documentation (if appropriate). ## Tests New `FluentNodeManagerBasePipelineTests`: nodes authored in `ConfigureAsync` get registered, the Objects-folder reference is mirrored into `externalReferences`, `OnNodeAdded` fires once, and the builder is sealed afterwards. Local run on net10.0, all passing: - Opc.Ua.Server.Tests (Fluent/Hosting/StateMachine/RuntimeNodeSet filter): 1286 - Opc.Ua.SourceGeneration.Core.Tests (NodeManager filter): 40 - Opc.Ua.Di.Tests 426, ISA95 136, Positioning 69, Robotics 636, Robotics.Intent 22, Vision 484 (+1 skipped), Vision.Intent 14, OpenUsd 921 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
e9313ef957 |
Only use GitHub action for CI and nightly, establish release-branch-only package publication (#4513)
## Description Two related changes to how the 2.0 line is built and shipped. ### 1. Release-branch-only package publication - `master` produces development previews only. - Stable packages can only be built from a canonical `release/<major>.<minor>` branch and are promoted manually from the exact signed candidate artifact. - Core packages use explicit `major.minor.patch` versions (`2.0.0`, `2.0.1`, then `2.1.0`); git height never changes a stable package patch. - XRegistry, WoT Connectivity, Vision, Robotics, Redundancy, Positioning, OpenUSD, ISA95, AI, DI, the relevant MCP extensions, and OpenUSD connector tools remain `-preview.N` while following the core numeric version. - nuget.org receives only non-`.Debug` package IDs; GitHub Packages retains the complete signed Release/Debug candidate. - `docs/ReleaseProcess.md` is a reproducible runbook for developers and agents, with each step stating its completion evidence. Promotion is source-bound and byte-checked: `release.yml` checks out nothing until the candidate SHA is known, and `assert-published-packages-match.ps1` proves that any version already on a feed holds *this* candidate's content before `--skip-duplicate` is allowed to skip it. Canonical release-branch naming is matched case-sensitively with no leading zeroes, and `version.json`'s `publicReleaseRefSpec`, both policy helpers and the copy `release.yml` inlines are pinned to one expression by tests. ### 2. PR CI moved to GitHub-hosted runners - `.github/workflows/buildandtest.yml` (**CI**) now owns pull-request validation outright, covering the Windows Debug/Release net48 + net10.0 solution builds and fast tests that only Azure Pipelines ran, alongside the existing Linux/macOS, all-target and NativeAOT legs. `build-and-test summary` remains the stable required check name. - `.github/workflows/nightly.yml` is a new **manual-only** (`workflow_dispatch`) full-scope workflow covering the Azure nightly workload: all solutions x seven target profiles x Debug/Release, the net472/netstandard profiles, Windows + Linux NativeAOT, and the long-running and durable tiers. - Shared deterministic discovery/batching lives in `.github/scripts/get-ci-matrix.ps1` with a local composite action, so both entry points expand the same workload manifest instead of duplicating a large embedded script. - `azure-pipelines.yml` is set to `pr: none`. - The standalone pump sample image workflow is folded into the consolidated **Images CI** workflow, which now builds all ten images and publishes an `images summary` rollup suitable as a required check. ## Related Issues No related issue provided. ## 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. ## Validation - `Opc.Ua.Tools.Tests` full `net10.0` suite: **959 passed, 1 skipped, 0 failed**. - Version/package-policy regression subset (`PackageVersionPolicyTests`): **60 passed, 0 failed**. - GitHub Actions **CI** run 35719807620: **149/149 jobs green** on GitHub-hosted runners. - Canonical release-branch rule verified against the pinned nbgv 3.7.115 by building throwaway branches: `release/2.0`, `release/0.9`, `release/10.42` -> `PublicRelease=True`; `release/02.0`, `release/2.00`, `Release/2.0` -> `False`. - Package identity gate verified against a live nuget.org package: served packages carry a repository `.signature.p7s`, so equality is decided on content excluding that part. Re-signing and zip reordering produce an identical digest; a one-byte content change does not. - Synthetic Release-only, Debug-only, combined, preview, duplicate, orphaned-symbol, mismatched-root and accidental-stable-family package sets exercise the shared validator. - Stable-release rehearsal in an isolated `release/2.0` clone: `Opc.Ua.Di` packed as `2.0.0-preview.6` depending on stable Core `2.0.0`; `Opc.Ua.Di.Client` likewise against stable Client `2.0.0`. - All workflow YAML, `version.json`, the package-policy PowerShell scripts and the internal release-runbook anchors parse. ## Known validation limits - Full `UA.slnx` `net48` and `net10.0` local validation remains blocked by pre-existing `NETSDK1005` and dependency-version failures in unmodified fuzz/sample projects; the corresponding checklist items intentionally stay open. - The TRX-driven test verdict added for the intermittent at-exit host stall is proven by `CiTestVerdictTests`; the stall itself has not recurred since. - `nightly.yml` has not yet been run against a full-scope Azure run for tuple-level parity — see below. ## Operational follow-up This remains a preparation PR targeting `master`. It does **not** authorize a release. Release sequence: 1. Merge this PR. 2. Create canonical `release/2.0`, advance `master` to `2.1.0-preview.{height}`, and open a separate `2.0.0` version PR against `release/2.0`. 3. Follow [`docs/ReleaseProcess.md`](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/docs/ReleaseProcess.md) for candidate dry-run, environment approval, promotion, tag/release finalization, and next-patch preview setup. CI migration steps still needing administrator access are tracked in [`docs/DeveloperGuide.md#migration-status`](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/docs/DeveloperGuide.md#migration-status): proving `nightly.yml` parity on a trusted SHA with the private fuzz corpus provisioned, turning off definition 14's service-side PR override, retiring the Azure full-scope schedules and push trigger, and retiring the duplicate `azure-pipelines-preview.yml` publisher. > **Before cutting `release/2.0`:** the `Release` ruleset requires the Azure context for *every* `refs/heads/release/*`, but a new 2.x branch inherits `pr: none`, so that context would never report and would block every PR into the new line. Exclude the new ref from that ruleset, or add a 2.x ruleset requiring `build-and-test summary`, as part of creating the branch. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
|
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 |
||
|
|
331a71ac2c |
Fix AI model version and inference argument handling (#4512)
## Summary
- Derive the AI server''s advertised specification version from
generated NodeSet metadata (new source-generated `ModelVersions.Target`
constant), so `AINodeManager.SpecificationVersion` can no longer drift
from the shipped model.
- Preserve inference parameters through inline, asynchronous, fallback,
and oversized-transfer execution instead of silently discarding them.
- Reject unsupported `PayloadUri` input explicitly with
`BadNotSupported` before any backend call, job creation, or transfer
allocation.
- Apply the supported `temperature` / `max_tokens` / `top_p` parameters
in both built-in backends via a shared parser.
Fixes #4505
Fixes #4506
## Review follow-ups (
|
||
|
|
02fb72e2d3 |
Support guid/opaque nodeids in NodeSet import (#4507)
# Description
`NodeSetToModelDesign.ImportNode` only stored Numeric and String NodeId
identifiers. Guid and Opaque identifiers (both valid per OPC 10000-3
5.2.2) fell through to an `Information` log line, so the node was
registered in `NodesByQName`/`NodesById` with no identifier at all and
code generation continued against a node whose identity had been
discarded.
The `ModelDesign` schema has no representation for Guid/Opaque
identifiers (`NodeDesign` exposes only `NumericId`/`StringId`), so the
import now fails loudly instead of emitting output that looks correct.
- **Importer**: the fall-through branches in `ImportNode` are replaced
with an `InvalidDataException` naming the NodeId, browse name and
identifier type. This matches the existing unsupported-input handling in
the file and surfaces through `ModelCompilation` as a generator
diagnostic.
- **Logger removal**: `m_logger` had no remaining use in
`NodeSetToModelDesign` and was removed along with its
`Microsoft.Extensions.Logging` import.
- **Tests**: `ImportUnsupportedNodeIdTypeThrowsInvalidDataException`
covers Guid, empty Guid and Opaque identifiers via an in-memory NodeSet.
- **Docs**: the source generator readme states the NodeSet2 identifier
restriction.
```csharp
else
{
// The model design can only carry numeric and string identifiers.
// Guid and Opaque identifiers (OPC 10000-3 5.2.2) would be dropped
// and code would be generated for a node without any identity, so
// the import fails instead of silently producing wrong output.
throw new InvalidDataException(
$"NodeId ({input.NodeId}) of node '{input.BrowseName}' uses the " +
$"{nodeId.IdType} identifier type which is not supported by the model " +
"design. Only Numeric and String identifiers can be imported.");
}
```
Note on behaviour: a null/unspecified NodeId parses as a Numeric
identifier (`i=0`) and is still handled by the numeric branch, so the
removed `IsNull` branch was only ever reachable for an empty Guid or
Opaque identifier, which is now reported as unsupported.
## Related Issues
## Checklist
_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes #4504
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
||
|
|
9f14ddf5de |
Preserve non-child hierarchical references during NodeSet import (#4491)
# Description Preserve forward and inverse same-namespace hierarchical references that are not already represented by the NodeSet importer's parent/child relationship. A reference is implicit only when both its endpoints and its exact reference type match the selected ownership relationship; additional links do not reparent their targets. Also fix the generated-reference duplication that blocked the previous attempt in #4473. Inherited and declared `NodeDesign` objects can differ structurally while resolving to the same runtime reference. Deduplicating by the emitted reference-type ID, direction, and target ID prevents identical `AddReference` calls from throwing during Robotics/OpenUSD startup. - Add 21 importer/generator regression cases, including a minimized recursive-placeholder reproduction of the startup failure. - Strengthen the reference-server test to check the original missing forward links, inverse counterparts, subtype filtering, and unchanged ownership. - Document ownership versus graph references in `docs/NodeManagers.md`. ## Validation Confirmed the missing-reference cases fail before the importer fix, and the minimized duplicate-reference case fails before the generator fix. The original Robotics/OpenUSD fixture passed before the changes, failed with the importer-only candidate, and passes with both fixes. | Suite | net10.0 | net48 | | --- | --- | --- | | SourceGeneration.Core.Tests | 3,961 passed; 8 existing skips | 3,961 passed; 8 existing skips | | SourceGeneration.Stack.Tests | 94 passed | 91 passed | | SourceGeneration.Tests | 185 passed | 185 passed | | Di.Tests, including all 17 RobotOpenUsdE2eTests | 425 passed | 425 passed | | ReferenceServerTests | 45 passed | 45 passed | Executed using `CustomTestTarget` with `dotnet test tools/SourceGeneration.slnx`, the Di test project, and the `ReferenceServerTests` filter in the Server test project. Scoped whitespace/style checks and `git diff --check` also passed for the changed code. Pre-existing analyzer diagnostics outside the changes were left untouched. The full `UA.slnx` test suite has not been run locally. CI and CodeQL results remain pending. ## Related Issues - Fixes #4484. - Addresses the hierarchical-reference limitation deliberately deferred in #4473. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
|
e639932b95 |
Fix source generator defects from the Client/Types/SourceGeneration audit (#4473)
# Description
Fixes the SourceGeneration findings of a read-only bug audit of
`a05a36055` — sections 15 (host generators, Roslyn side), 16
(SourceGeneration.Core schema / ModelDesign loader) and 17
(SourceGeneration.Core generators). 36 of the 39 findings are fixed;
each has a regression test.
### Generated-code correctness
- **State machine NodeId constants** were emitted as `const uint` even
when the aliased `Objects` constant is a `string`, so every model using
string NodeIds with a `FiniteStateMachineType` subtype failed with
CS0029. The constant type now follows the aliased node, looked up from
the same source `NodeIdGenerator` emits from — top-level nodes **plus
each node's instance hierarchy**, since a state or transition only ever
exists as a hierarchy entry.
- `NodeIdGenerator` emitted string identifiers into C# string literals
without escaping them.
- Structure field names are authored data, so they can be C# keywords or
collide with the members every generated data type carries. Properties
and `{Type}Fields` members now go through a sanitizing,
collision-breaking mapping (including two siblings that sanitize to the
same identifier). **Wire names are untouched** — OPC 10000-3 §8.51
requires the StructureField name to match the BrowseName, and the
`DataMember`/encode/decode names still come from `Parameter.Name`.
- `NodeStateGenerator`'s copy-on-write cast now uses the fully qualified
data type name, which a cross-model structure needs.
- `GetScalarValueAsCode` emitted `(double)NaN` and the infinities, which
do not compile.
- `TemplateWriter` formatted with the current culture, so a machine with
a comma decimal separator emitted invalid generated source.
### Served schema documents
- The binary schema described a **different wire layout than the
generated `Encode`/`Decode`**: unions carried no
`SwitchField`/`SwitchValue`, and structures with optional fields carried
no encoding-mask presence bits. Both are now emitted per **OPC 10000-6
§5.2.7**, matching the layout the runtime `BsdSchemaGenerator` produces.
A structure with more than 32 optional fields now fails generation
instead of silently wrapping `1 << 32` back onto the first field's bit.
- Documentation text and attribute values are XML-escaped.
### ModelDesign loading and validation
- Identifiers pinned in a CSV were dropped whenever any other loaded
namespace declared the same symbolic name. Removal is now scoped to the
file's target namespace, and explicit `NumericId`s are keyed by
`(namespace, id)` so the same number in two namespaces is legal.
- Declaration order inside a design file no longer matters — base types
are imported and validated ahead of their subtypes.
- OptionSet `OptionSetValues` were truncated at bit 31 (signed shift)
and bits 32–63 of a 64-bit option set were never emitted.
- An authored `ValueRank` is no longer overwritten by the shape of the
default value (OPC 10000-3 §6.2.8: a ValueRank may only be further
restricted), and a VariableType data type may be narrowed any number of
levels deep rather than exactly one.
- NodeSet import: `AccessRestrictions="0"` means "no restrictions", not
`EncryptionRequired` (OPC 10000-3 §8.56) — an unrecognised mask stays
**fail-closed**; a NodeSet without `<Models>` reports what is wrong; an
optional `<References>` no longer throws; the parent/child reference
scan looks at the parent's references; and `IsTypeOf` terminates on the
self-referential root reference types NodeSets routinely declare.
- Version comparison is numeric per component (ordinally `"1.05.9"`
sorts above `"1.05.10"`) and now lives on `SemVer`, so the NodeSet
collection and the dependency tie-break cannot pick different winners; a
publication-date version no longer outranks a real one.
### Host generator
- `[DataType]` on an `enum` is matched again (an enum is not a
`TypeDeclarationSyntax`), MODELGEN003 renders its arguments instead of
the raw template, and the Types hint name keeps the namespace's dots so
`A.BC` and `AB.C` cannot claim the same one.
- Compilation-derived pipeline stages carry value comparers, so editing
a file no longer re-runs the entire model generation on every keystroke.
- A NodeSet that fails to load skips that file rather than every
remaining one; a file listed twice as an `AdditionalFile` is deduped;
superseded NodeSet versions are still recognised as NodeSet inputs and
not generated a second time by the ModelDesign pass.
- `LogLevel.Critical` maps to a diagnostic instead of being silently
dropped.
### Client
- `EventRecordFieldReaders` gained the readers the event record
generator needs (including `Variant`, enumerations and the array forms).
Fields whose type had no reader were declared on the record but never
populated, so they read as `null`/`default` for every event.
## Not fixed (deliberately)
- **Forward same-namespace hierarchical references in
`NodeSetToModelDesign.ImportReferences`** — the audit flags the
inverse-only handling as asymmetric. Making it symmetric compiles and
passes every generator unit test, but regresses the address space: all
17 `RobotOpenUsdE2eTests` fail in `OneTimeSetUpAsync` because the hosted
server never finishes starting. Reverted, with a comment at the site
recording why.
- **§17.10 and §15.10** (both low + PLAUSIBLE) — each needs a wider
refactor than this PR: a consistent NodeId-constant rename across ~6
cross-reference sites, and replacing `Location` with a serializable
equivalent for incremental cache equality.
## Spec cross-check
The encoding/schema changes were checked against the specification: OPC
10000-6 §5.2.7 (32-bit EncodingMask, one bit per optional field in
order, padded), the union `SwitchField` encoding, OPC 10000-3 §8.56
(AccessRestrictionType bit mask), §6.2.8 and §5.6.2/§5.6.5 (ValueRank),
§8.51 (StructureField name == BrowseName).
`GenerateStackTests.OptionalFieldBitsAgreeBetweenBsdAndGeneratedEncoder`
pins the BSD presence-bit order against the generated encoder's
`LogRecordFields` so the two cannot drift apart again.
## Related Issues
_No tracking issue — this PR implements the SourceGeneration sections of
a bug audit. Happy to open one if maintainers prefer._
## Checklist
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
> Note on the test checkbox: verified on **net10.0 only** — `UA.slnx`
builds clean (0 warnings, 0 errors) and
`Opc.Ua.SourceGeneration.Core.Tests` (3915),
`Opc.Ua.SourceGeneration.Tests` (184), `Opc.Ua.Di.Tests` (425, including
the `RobotOpenUsdE2eTests` end-to-end fixture that catches address-space
content changes) and the `EventRecordFieldReaders` tests (16) all pass.
.NET Framework has **not** been run locally; CI needs to confirm it.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
|
||
|
|
f5ad124891 |
Additional fuzzing plus fixes for every defect the campaigns found (#4459)
# Description
Brings the stack's fuzz testing to parity with the fork
(`aio-opcua-sdk`), then fixes every defect the resulting campaigns
exposed.
The fork's fuzzing was used as the behavioural baseline: all **28
fork-local callbacks** and **13 OneFuzz registrations** are now
accounted for by an explicit mapping, not just by a larger target count.
The work extends the existing SharpFuzz/NUnit infrastructure rather than
introducing another framework.
### Coverage
- **84 continuous libFuzzer targets** and **81 AFL adapters** across
Encoders, Certificates, Network and PubSub.
- **PubSub area integrated** (Corpus/Tools/Tests siblings, solution
entries) — it previously had host source only and was absent from the
solution and replay infrastructure.
- Segmented binary adapters, binary-to-JSON and XML-to-JSON conversions,
standalone certificate loading, both chain-parser variants, CRL encoding
and canonical TBS round-trips.
- The 12 original fork message inputs are retained byte-identically
alongside richer deterministic generators; the three historical
crash/slow/timeout reproducers are unchanged (verified identical by
SHA-256).
### Production fixes found by fuzzing
These are real robustness and data-integrity bugs, each with a retained
regression input:
- `UadpBinaryReader.Advance` accepted attacker-controlled lengths on the
receive path. Declared payload sizes are now authoritative bounded
sections, with negative-count rejection and overflow-safe reassembly
arithmetic.
- PubSub JSON decoding leaked `InvalidOperationException` for invalid
UTF-8.
- `JsonEncoder.WriteExtensionObject` leaked
`System.Text.Json.JsonReaderException`.
- `JsonEncoder` silently emitted a lossy `{}` for an `ExtensionObject`
whose TypeId could not be represented in JSON.
- `JsonDecoder` wrapped a bodyless `ExtensionObject` envelope as a raw
JSON body instead of preserving `ExtensionObjectEncoding.None`.
- `Node.Encode` lost the null/empty collection distinction.
- Binary/XML Variant matrix dimensions are prevalidated rather than
relying on a caught `ArgumentException`.
- `X509CRL.ReadTime` pre-validates strict RFC 5280 whole-second Zulu
time forms.
- `Utils.ParseCertificateChainBlob` disposes correctly on partial-chain
failure.
- **`nodeid.dict` was unloadable by libFuzzer** (empty token, unescaped
quote). libFuzzer rejects the whole file on one bad line, so four parser
targets silently executed **zero** inputs. Fixed and now guarded by
dictionary validation.
### Infrastructure
- `fuzzing/fuzz-targets.json` manifest recording adapters, corpora,
dictionaries, oracles and fork-behaviour mappings, with
`validate-fuzz-manifest.ps1` and **27 negative controls**.
- Strict host/tools discovery and replay: `--list`, `--replay`, non-zero
exits on unknown targets, missing or empty corpora, and failed callbacks
(playback previously logged failures and returned success).
- A real process watchdog for the timeout/slow regressions — NUnit
`CancelAfter` cannot interrupt a synchronous fuzz callback.
- Runner scripts made non-destructive (unique work directories, no
generic `bin`/findings deletion) and corrected to the native driver's
single-`--target_arg` apphost contract.
- Opt-in `OneFuzz=true` class-library publication producing a
relocatable, **uninstrumented** drop, validated by exact callback
discovery, dependency closure, corpora and dictionaries.
- CI: fuzz replay matrix plus a public drop gate, triggered by corpus-,
dictionary- and script-only changes.
### Verification
| Gate | Result |
|---|---|
| Bounded campaigns, all 84 targets (1,000 inputs, seed 1) | 84/84,
84,000 executions, zero findings |
| OneFuzz drop | 84/84 published callbacks replayed, zero skips |
| Fuzz replay net10.0 / net8.0 / net9.0 | 9,470 passed each, 0 failed |
| Fuzz replay net48 | 8,996 passed, 0 failed (Network/PubSub are
modern-only) |
| `Opc.Ua.Types.Tests` / `Opc.Ua.Core.Tests` | 9,800 and 4,442 passed, 0
failed |
| Retained regression assets | 132 `crash-*` inputs replayed by every
target each run |
### Known gaps (deliberately not closed)
- **Fidelity is not enforced against the pipeline crash corpus.** The
differential binary-to-JSON targets assert both robustness (no unhandled
exception, expected of arbitrary bytes) and fidelity (a decoded value
re-encodes equivalently, a property of well formed values). The pipeline
supplies 22,293 blobs collected for the targets that existed before this
PR, none of which asserted fidelity; 522 of them violate it, and that
count did not move across two rounds of genuine fixes because each fix
only lets the mutator reach the next representability gap. Rather than
weaken the assertion, the two contracts are separated: fidelity failures
raise `EncodingFidelityException`; curated assets under `Assets/Repo`,
the curated `Testcases` corpus and every continuous libFuzzer campaign
stay strictly enforced, while externally supplied assets are gated on
robustness only and their fidelity findings are reported with counts and
bounded base64 reproducers.
- **Internal MSFT OneFuzz canary has not been run.** It needs an
owner-supplied ownership profile, pipeline authorisation and corpus
snapshot; none were available. The drop builds and validates locally and
records `serviceCanaryPerformed: false`. A build or upload is not a
successful worker run.
- **Legacy JSON wire forms are a partial mapping.** The fork accepts its
four original `TypeId`/`Body` messages; this stack rejects that retired
envelope (measured JSON-only control: fork `JsonEncoder` 630/1356 lines
vs 0/1007 here). Recorded as an explicit unresolved mapping rather than
an exemption. `LegacyNonReversible` is a RawData-derived profile and is
not counted as independent coverage.
## Related Issues
_No tracking issue was supplied for this work; happy to open one and
link it if maintainers prefer._
- Fixes #github-issue-number, ...
## Checklist
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
> Note on the checklist: the fuzz replay matrix, `Opc.Ua.Types.Tests`
and `Opc.Ua.Core.Tests` were run locally on **net48** and **net10.0**
(plus net8.0/net9.0) and all passed. CI and CodeQL results are not yet
available for this branch, so those boxes are left unchecked. The CLA
box is for the author to confirm.
|
||
|
|
ececea15c8 |
Reimplement the GDS ApplicationsNodeManager as a generated fluent node manager (#4434)
# Description Builds on #4428, which is now merged; this branch is rebased onto master. The GDS `ApplicationsNodeManager` becomes a source-generated `[NodeManager]`: the companion model, its loader and the fluent plumbing are emitted, and what stays hand-written is the behaviour. Getting there needed three things from the shared surface, which are the first two commits. ## Commits **1. Add the fluent hooks a hand-written manager needs at startup** - `OnReadRolePermissions` / `OnReadUserRolePermissions` on the node builder. Every other `NodeState` `On*` hook was already fluent; these were not, and a server that grants access on something the static model cannot express has to compute the permission set per request. - `Node<TState>(TState)` returns a builder over a node the caller already holds, for nodes materialised after `Configure` resolved its graph. - `OnAddressSpaceReadyAsync` on `FluentNodeManagerBase` — the one seam between the address space existing and the wiring being applied, for managers whose wiring depends on asynchronous setup. `Configure` is a `partial void` and cannot await. `DiNodeManager` had already invented the hook privately, so its declaration becomes an `override`. **2. Let a `[NodeManager]` bind to a model a referenced assembly supplies** A model belongs to the assembly that emits its types, which leaves a node manager for it homeless whenever that assembly cannot reference `Opc.Ua.Server` — a model-only package shared with clients, for instance. Binding was rejected twice over: the design was skipped before the binding was matched (`MODELGEN010`), and fluent-accessors-only mode refused to run alongside manager generation (`MODELGEN014`). The binding is now matched before the skip decision, and a matched binding puts that design into accessors-only mode. A design nothing binds to is still skipped, so this cannot start duplicating types by accident. The emitted manager also gains what a real manager needs: `DefaultNamespaceUris()` plus a protected constructor taking a replacement set; `GenerateDefaultConstructor=false` to suppress the two-argument form; its own logger category; and the call to `OnAddressSpaceReadyAsync`. Generated fluent accessors also stop re-enabling nullable warnings — a bare `#nullable enable` undid the shared header's `disable warnings`, so any model with a structure-typed method argument reported CS8600 on every generated `out T` in a project that builds warnings as errors. **3. Reimplement the GDS `ApplicationsNodeManager` as a generated node manager** The design stays owned by `Opc.Ua.Gds.Common`; the server assembly only binds a manager to it. **NodeIds do not move.** `NamespaceIndexes[0]` must stay the application record namespace — it is the one the application database, the certificate request store and the base allocator mint ids in — so the constructor passes the namespace order explicitly rather than adopting the generated model-first default. A test pins both ends against a live server. Four overrides go, leaving `OnAddressSpaceReadyAsync` and `Dispose(bool)` — no address-space overrides at all: | Removed | Why it was unnecessary | | --- | --- | | `New` | The base allocator already mints into `NamespaceIndexes[0]`, with a thread-safe counter. The override existed only because `Create(assignNodeIds: true)` asks the factory with the declaration id still set; `AssignInstanceNodeId` nulls it and re-asks, documented as being there for exactly this allocator. | | `AddBehaviourToPredefinedNodeAsync` | Its `AuthorizationServiceType` branch re-wired a node that was already wired, and its `KeyCredentialServiceType` branch had nothing to wire — the model ships that folder empty. | | `GetManagerHandleAsync`, `ValidateNodeAsync` | Verbatim reimplementations of the base that also blocked the fluent base's virtual-node support. | | `DeleteAddressSpaceAsync` | A `// TBD` passthrough. | Everything else — all sixteen directory methods with their self-administration permissions, the certificate groups' concrete types and trust-list flags, and the authorization service (created through this PR's `Add` surface and wired in the same place) — is one `Configure` pass. **4. Bind the certificate groups from the `Configure` pass** Acquiring a certificate group and binding it to its nodes are different kinds of work: the first is I/O (`InitAsync` opens certificate stores and creates CA certificates), the second only touches the address space. Each now runs in the phase it belongs to, which puts the whole of the wiring in one place. Ownership no longer rides on the binding. A group joins `m_ownedCertificateGroups` the moment it is constructed, before `InitAsync` can throw, so a group that fails half way through is still disposed — which the previous code could not manage, keying ownership off a dictionary it filled only after initialization succeeded. > Commit 3's message says this move is impossible and blames `PushTest`. **That was a wrong diagnosis, and this commit supersedes it.** Two real defects were hiding behind those failures: - **`Create(..., NodeId.Null, ..., assignNodeIds: false)` does not null the NodeId.** `NodeState.Create` overrides it only when handed a non-null one, so a custom group node kept the namespace-0 id of its type declaration. `AssignInstanceNodeId` hid this by re-asking the factory with the id nulled; the builder validates instead, and rejected it. The node is now created with an id in the manager's own namespace, as the `Default` authorization service already was. - **The custom certificate group node was not browseable.** It answered `GetCertificateGroups` and `ReadNode`, but a type-filtered recursive browse from the Objects folder never returned it — which OPC 10000-12 §7.8.2 requires of a certificate group. Staging the node through the builder gives it the reference it was missing. ### Two `PushTest` methods change — please review these closely The second defect had been masking what those tests actually did. Selecting the group positionally, `groups[3]` resolved to the server's **own `ServerConfiguration` group** while the custom one stayed invisible, so `UpdateTrustListOfCustomGroupAsync` and `AddRemoveCertOfCustomGroupAsync` were exercising a *transactional* trust list under a custom-group name. Browse results, before and after: | # | Before (baseline) | After | | --- | --- | --- | | 0-2 | GDS Directory groups (`ns=3`) | GDS Directory groups (`ns=3`) | | 3 | **`DefaultApplicationGroup` `i=14156`** — the server's own | **`MyCustomGroup` `ns=2`** | | 4 | — | `DefaultApplicationGroup` `i=14156` | They now resolve the group by browse name — which asserts it is browseable at all — and expect the immediate-apply semantics the GDS actually gives the groups of its own `Directory`: `CloseAndUpdate` writes the stores at once and reports `applyChangesRequired == false`, so the `ApplyChanges` calls go away (they would return `Bad_NothingToDo`). The read-back assertions are what prove the writes landed, and they are unchanged. This is a genuine loss of the *transactional* trust-list coverage those two methods were accidentally providing — the server's own group is still covered by the other `PushTest` methods, which target it deliberately. **5. Select `GetCertificates` results by type instead of by position** `GetCertificatesAsync` checked entry zero of a parallel-array result. Which entry that is depends on the order the server reports its certificate types in, an empty result throws `IndexOutOfRange` rather than failing an assertion, and a second type going missing or unparseable would not be noticed. Every returned pair is now checked, and a failure names the certificate type. **6. Drop the `.Common` suffix from the three GDS projects** Every other companion specification ships as `<Family>` / `<Family>.Client` / `<Family>.Server` — Di, ISA95, Positioning, Robotics, Vision, WotCon, XRegistry. GDS was the exception, with a `.Common` suffix that named nothing. | Before | After | | --- | --- | | `src/Opc.Ua.Gds.Common` | `src/Opc.Ua.Gds` | | `src/Opc.Ua.Gds.Client.Common` | `src/Opc.Ua.Gds.Client` | | `src/Opc.Ua.Gds.Server.Common` | `src/Opc.Ua.Gds.Server` | **No source changes were needed.** `RootNamespace` was already the final name in all three projects — the namespaces have always been `Opc.Ua.Gds`, `Opc.Ua.Gds.Client` and `Opc.Ua.Gds.Server`, and only the assembly and package ids disagreed. Nothing compiling against these assemblies changes a `using`. The package ids do change, which is consumer-visible, so `docs/migrate/2.0.x/packages.md` gains a renamed-packages section — it is the document that undertakes to cover NuGet renames — and the migration skill's package table gains the two rows a 1.5.378 consumer needs. Build infrastructure that names the projects or their output moves with them: both solution files, `expected-packages.txt` (re-sorted — it is maintained alphabetically), both signing manifests, the reference-server Dockerfile, the metapackage nuspecs, and the analyzer's package dependencies. `preview-pack.slnx` is derived from `UA.slnx` and needs no edit. The migration analyzer's shim tree mirrors the source layout by its own documented convention, so `Gds.Client.Common/` becomes `Gds.Client/`. Prose naming past work is left alone — plan 23 still records that its deferral happened during the "GDS Client.Common modernization", which is the name that effort had. ## Validation `net10.0`, on master + these four commits, from a clean GDS PKI: - `UA.slnx`: 0 errors, 0 warnings in changed files (the solution's 30 pre-existing warning lines are CA1861/CA1850/CA1307 in `XRegistry`, `Types.Tests` and `WotCon.Tests` — same rules and projects as the base). - `Opc.Ua.Gds.Tests`: 1,117 passed, 0 failed, 54 skipped — identical to the base branch. - `Opc.Ua.Server.Tests`: 4,961 passed, 0 failed, 5 skipped (measured before commit 4, which touches only the GDS assembly and its tests). - `Opc.Ua.SourceGeneration.Tests`: 169 passed. - `Opc.Ua.SourceGeneration.Core.Tests`, NodeManager/Generators/Fluent: 84 passed; `GenerateCode`: 21 passed, 4 skipped. - `Opc.Ua.MigrationAnalyzer.Core.Tests`: 10 passed, 3 skipped. - After the rename: assemblies build as `Opc.Ua.Gds.dll` / `.Client.dll` / `.Server.dll`, with no `*.Common.dll` produced anywhere. - Commits 1-3 build standalone. - `Opc.Ua.Gds.Server.Common` also builds for `net48` and `netstandard2.1` (checked before the rebase onto #4428). Not yet run: the full `Opc.Ua.SourceGeneration.Core.Tests` suite (~3,800), and the older target frameworks since the rebase. ## Checklist - [x] Existing tests pass - [x] New tests cover the new surface (role permissions, resolved-node builder, referenced-model binding, constructor/namespace control, GDS namespace invariant, custom-group node namespace) - [ ] Full Core.Tests sweep and net48/netstandard2.1 re-run after the rebase 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
|
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>
|
||
|
|
668310f21f |
Make the fluent node manager's configure/seal path genuinely async (#4442)
# Description The fluent node-manager pipeline had two synchronous chokepoints, and two independent efforts have now hit both of them. `Configure` is a `partial void`, so it cannot `await`. A manager whose wiring depends on asynchronous setup had to invent a second hook outside the pipeline, create its own builder there, and hand-roll `CreateFluentBuilder(...).Configure(...).Seal()` inside it. `Seal()` was synchronous, so nothing a registration staged during `Configure` could be completed by awaiting. Registrations that needed to await either blocked — `EventSourceRegistry` did `.GetAwaiter().GetResult()` on the manager's root-notifier registration — or could not be expressed at all. ## What changed **`FluentNodeManagerBase.ConfigureAsync(INodeManagerBuilder, CancellationToken)`** is the awaitable wiring seam. It runs once per activation, with the manager's attached builder, immediately *before* the synchronous `Configure` partial(s) — so instances it materialises are in the address space by the time `Configure` wires callbacks against them. The default implementation is a no-op. It is a `virtual` method rather than a second `partial` declaration because a partial method can be optional (`partial void`, no return value) or awaitable (an extended partial method, which must be implemented) — not both, and the generator would otherwise have to detect which one the user declared. `partial void Configure(INodeManagerBuilder)` keeps working unchanged; a manager uses either hook or both. **`NodeManagerBuilder.Seal()` becomes `SealAsync(CancellationToken)`.** Sealing is now the single point where registrations that could not finish inside `Configure` get their turn to await. Every seal site awaits it, so a later change can move registry *setup* onto the same seam. **`DiNodeManager` now runs the standard pipeline itself** — create and attach the builder, `await ConfigureAsync`, `RegisterAuthoredNodesAsync`, `CompleteConfigureAsync`, the post-setup runner, then `await builder.SealAsync` — instead of handing subclasses a bare callback and leaving them to build and seal on their own. DI subclasses gain node registration and the reverse-reference pass, which they previously skipped. Sealing deliberately comes **after** the post-setup runner. The fluent registries are owned by the manager, not by a single builder, and sealing starts them — so sealing first locks post-setup configurators out of registering simulation loops of their own, which is exactly what `AddRobotics`' configurators do. Sealing before the runner makes `MinimalRobotServer` fail to start with *"Cannot add a simulation loop after the registry has started"*. Worth knowing for follow-up work: with several builders attached to one manager, the first seal freezes the shared registry for all of them. ## Workarounds removed - **`DiNodeManager.OnAddressSpaceReadyAsync`** — deleted. Its three overriders (`RoboticsNodeManager`, and the `PumpNodeManager` / `GeneratorNodeManager` samples) now override `ConfigureAsync` and no longer hand-roll `CreateFluentBuilder(...).Configure(Configure).Seal()`. - **`EventSourceRegistry`'s `.GetAwaiter().GetResult()`** on `AddRootNotifierAsync` — gone. `Publish(..., RegisterAsRootNotifier: true)` now stages the registration and `SealAsync` drains it, so the wait happens on an awaited path. A source whose root-notifier registration fails is rolled out of the registry before the failure surfaces, and a registry disposed before its builder sealed drops what it staged. - **`Isa95NodeManager.ConfigureStatusEvents()`** — the synchronous second builder pass is now `ConfigureStatusEventsAsync(CancellationToken)`, awaited from `CreateAddressSpaceAsync`. (`ConfigureCatalogChanges()` stays synchronous: it starts a background task and never seals a builder, so there was nothing to convert.) - **`IRoboticsBuildContext.Seal()`** — now `SealAsync(CancellationToken)`. These really were `NodeManagerBuilder.Seal()` calls behind a same-named method. The state transition still happens under the context lock; the builder's asynchronous completion runs outside it, so no `await` is taken while the lock is held. - The Pump / Generator incremental registration callbacks (`onRegistered`) changed from `Action<T>` to `Func<T, CancellationToken, ValueTask>` so their per-instance builder pass can await its own seal. No sync-over-async was introduced anywhere. `Dispose(bool)` is unchanged and stays non-blocking; teardown remains in `DeleteAddressSpaceAsync`. `NodeManagerBuilder.Seal()` and `IRoboticsBuildContext.Seal()` are removed rather than kept as synchronous overloads. A synchronous seal that silently skipped the asynchronous completion would reintroduce exactly the failure mode this change exists to remove. ## Generator `NodeManagerTemplates` emits, in order: predefined nodes load → `await ConfigureAsync(__m_builder, cancellationToken)` → `Configure(__m_builder)` and the typed `Configure` → `RegisterAuthoredNodesAsync` → `CompleteConfigureAsync` → `await SealConfigurationAsync(__m_builder, cancellationToken)`. `NodeManagerGeneratorTests` asserts that order literally, and that neither `__m_builder.Seal()` nor the synchronous `SealConfiguration(__m_builder)` is emitted. ### Merging with #4432 #4432 landed on `master` while this was in review and reworked the same seam from the other side: it split `Seal()` into `SealGraphAuthoring()` + `StartSimulations()` so a manager can replay `NotifyNodeAdded` *between* the two halves — sealing first stops a lifecycle handler authoring nodes nothing would register, starting the simulations last stops a simulated value change preceding its own node''s `OnNodeAdded` — and added `FluentNodeManagerBase.SealConfiguration(builder)` as the one call the generator emits. Both intents are kept. That split is precisely why the asynchronous completion could not simply live inside `SealAsync`: a manager reaching the seam through the split path would silently skip it, which is the failure mode this PR exists to remove. So the asynchronous half became its own step: - `NodeManagerBuilder.CompleteSealAsync(ct)` drains the staged root-notifier registrations, then starts the simulations. `SealAsync(ct)` is `SealGraphAuthoring()` followed by it. - #4432''s synchronous `StartSimulations()` is gone, folded into `CompleteSealAsync`. It had exactly one production caller after the merge, and leaving it standing would have kept a second, synchronous way to activate a builder that skips the staged registrations — the very hazard this PR removes. Activation is now one step with one entry point; the *split* itself stays, because the replay has to sit between the halves. - `SealConfiguration` becomes `SealConfigurationAsync(builder, ct)`: seal the graph, replay `NotifyNodeAdded`, then `CompleteSealAsync`. Activation still happens last, so #4432''s ordering guarantee holds unchanged. ## Consumers unblocked - **#4434 ("Reimplement the GDS ApplicationsNodeManager as a generated fluent node manager")** adds `OnAddressSpaceReadyAsync` to `FluentNodeManagerBase` purely because, in its own words, *"`Configure` is a `partial void` and cannot await"*. That hook is unnecessary once `ConfigureAsync` exists — the GDS manager overrides `ConfigureAsync` instead, and the ordering it wants (predefined nodes → async setup → `Configure`) is what the generator now emits. #4434 has not merged yet, so it can drop its addition rather than have it simplified afterwards. - **Branch `romanett/fluent-node-behaviors`** moved `SimulationRegistry`, `EventSourceRegistry`, `MonitoredSourceRegistry` and the alarm wiring onto `AttachToType`/`Attach` for *release* only, because activation ran from `CompleteConfigureAsync` and most managers only called the synchronous `Seal()`. Every seal site now reaches `SealAsync`, so registry and alarm *setup* can move onto behavior activation too — the remaining half of that work. ## Tests - New `DiNodeManagerConfigureAsyncTests` drives the real `DiNodeManager.CreateAddressSpaceAsync` and asserts the hook runs once with an attached builder, that nodes it stages are registered, that their references to externally owned nodes are published, and that the builder is sealed before `CreateAddressSpaceAsync` returns. - `PublishTests` now pins the root-notifier deferral contract: registration is staged during `Configure`, lands only on the seal, and draining twice does not double-register. - Every `Seal()` call site in the test suites became `await ...SealAsync()`, including the three #4432 added in `NodeSetImportBuilderTests` and `SimulationBuilderExtensionsTests`. ## Related Issues No tracking issue was opened for this; it is the shared prerequisite extracted from #4434 and branch `romanett/fluent-node-behaviors`, both of which had independently worked around the same synchrony. Happy to open one if maintainers want the ADR trail. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ### What was actually run locally `UA.slnx` builds clean. Post-merge on **net10.0**: `Opc.Ua.Server.Tests` 5020/0 and `Opc.Ua.SourceGeneration.Core.Tests` 3813/0. Pre-merge the same run plus `Opc.Ua.Di.Tests` 425/0, `Opc.Ua.Robotics.Tests` 627/0, `Opc.Ua.ISA95.Tests` 136/0, `Opc.Ua.Positioning.Tests` and `Opc.Ua.Robotics.Intent.Tests` were all green; `MinimalRobotServer` was also started directly to confirm the hosted robotics path comes up. Three caveats, so the unchecked box above is not a mystery: the full suite was **not** run against .NET Framework locally; `Opc.Ua.OpenUsd.Tests` has one pre-existing failure on this machine (`StructuredCartesianCoordinatesAreAccepted` expects `"1.5"` and gets `"1,5"` — a German-locale decimal separator, unrelated to this change); and one pre-existing `CA1861` warning comes from `NodeSetImportIntegrationTests.cs`, which is byte-identical to `master` on this branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
da7412c429 |
Add native xRegistry events and distinct Resource versions (#4396)
## Summary - support the xRegistry 0.6.0 distinct Resource/Version hierarchy: `Registry -> Group -> Resource -> Versions -> Version` - instantiate stable logical Resource nodes separately from exact Version nodes while preserving existing Version NodeIds - delegate logical Resource FileType operations and Version-view metadata to the selected default Version with handle pinning and shared writer reservations - make Resource and Version delete behavior role-stable and use `MetaEpoch` / `Epoch` respectively - use stable Resource SourceNodes, exact Version SourceNodes and the `Server -> Registry -> Group -> Resource -> Version` notifier chain - keep a model-metadata-gated client fallback for 0.5 flat Servers; 0.6 Servers expose no legacy aliases - target `HasWoTProjection` at the real stable logical Resource node - use `AsyncCustomNodeManager` for the registration, fast-path, and federation managers, with awaited lifecycle, node mutations, event delivery, and cleanup - handle startup rollback, reentrant deletion callbacks, undisclosed file handles, and retryable cleanup without blocking unrelated file operations ## Authoritative dependencies - companion specification: OPCF-Members/spec-drafts#32 at `d3399fca025025a73e5a6b599f850a81d985c060` - OPC UA binding: xregistry/spec#568 - xRegistry event semantics: xregistry/spec#563, merged as `66ee31e47fbadff6ab982ae1516f7d9065e46b64` - companion NodeSet SHA-256: `369f5f3f1814b333e4eeb52bfb4b6a730cdff372505a5f91a54c94f0016ac4cf` ## Review feedback - keep the WoT Connectivity information-model designation at `1.1`; draft revisions update the publication date without changing the model version - eliminate all 198 unique CA1861 diagnostics across 54 files and remove seven obsolete project suppressions - fix the analyzer-clean touched-path CA1307 and the hierarchy follow-up CA1846/CA1851 diagnostics without dropping legacy-TFM compatibility - recover the reconcile queue after callback faults/cancellation and continue accepted pending changes - remove migration guidance for the xRegistry managers, which are new in 2.0 ## Compatibility - public WoT Connectivity model designation remains `1.1` - generated event-record ABI, constructors, options, NodeIds, and stored documents remain compatible - the generic xRegistry managers are new in 2.0 and implement `IAsyncNodeManager`; use `IAsyncNodeManagerFactory` and the async lifecycle hooks (see `docs\XRegistry.md`) - persistence schema and document blobs remain compatible unless noted below ## Validation - merge validation at `980422f76` with upstream `e8bec27d5`: 454 targeted WoT/materialization/model/registry cases and all 327 XRegistry cases passed on both net10.0 and net48; all 29 net10 aggregation sample cases passed - feedback regressions at `308e8ba74`: 55 targeted queue and live registry event/projection/lifecycle cases passed on each of net10.0 and net48 - prior CI: all 266 checks passed at `6489a0908`; the unchanged Ubuntu Core certificate-counter test and dependent jobs passed on the one-time retry ([run 34117304035, attempt 2](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/34117304035/attempts/2)) - `Opc.Ua.XRegistry.Tests` — 327 passed on net10.0 and net48 for migration commit `32c914aaf` - `Opc.Ua.WotCon.Tests` net10.0 — 1348 passed - `Opc.Ua.WotCon.Bindings.Tests` net10.0 — 683 passed - `Opc.Ua.SourceGeneration.Core.Tests` net10.0 — 3807 passed, 8 skipped - `Opc.Ua.SourceGeneration.Tests` net10.0 — 145 passed - `Opc.Ua.SourceGeneration.Stack.Tests` net10.0 — 94 passed - affected XRegistry and WoT client/server projects build successfully - xRegistry NodeSet is byte-identical to the companion artifact - repository-wide CA1861 inventory — 198 unique diagnostics across 54 files reduced to zero - targeted changed-fixture validation — 1,098 passed, 3 skipped, 0 failed - constrained full `UA.slnx` net10.0 Release build — CA1861 = 0 - `Opc.Ua.XRegistry.Client` net472 and netstandard2.1 builds — 0 warnings, 0 errors - independent shared-array safety review — no significant issues - `git diff --check` The final review regressions cover concurrent reconcile/open, synthetic logical handle collisions across default switches, cross-session Close isolation, shared writer reservations, logical FileType property/change propagation, logical Labels delegation, role-fixed deletion, event sources, model-version gating, and stable `HasWoTProjection` targets. The companion specification and protocol binding are reviewed in the linked PRs above. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5c79a1ed-6dab-4522-a9fc-6548caa78ad5 Copilot-Session: b605797f-a97a-4cd0-8f12-016d33c2fd88 Copilot-Session: 7ef369a5-2307-4f71-8a11-15af0e72f240 Copilot-Session: e716d455-8397-4d22-9c5a-398847ab7317 |
||
|
|
30ed2390a8 |
Import NodeSet2 documents into a fluent node manager (#4432)
## What
`INodeManagerBuilder.Import(UANodeSet, INodeSetImportFactoryProvider?)`
lets a source-generated (or hand-written fluent) node manager overlay a
NodeSet2 document that is only known at runtime onto the model it was
generated from.
Every document imported during one `Configure` pass forms a single batch
that is linked **exactly once** after the pass. A node may therefore
declare a `ParentNodeId` that lives in another document of the batch, or
a node the manager already owns — which is what makes an overlay
possible. Imported nodes resolve through
`Node(...)`/`NodeFromTypeId(...)` immediately, so they can be wired in
the same pass; they are registered with the manager when
`CompleteConfigureAsync` runs.
### Public surface
- **+1 method**: `INodeManagerBuilder.Import(UANodeSet nodeSet,
INodeSetImportFactoryProvider? factoryProvider = null)`.
- **+3 contracts** in `Opc.Ua.Server.Nodes`:
`INodeSetImportFactoryProvider`, `INodeSetImportFactory`,
`NodeSetImportDiscriminator` (TypeDefinition / MethodDeclaration /
NodeId).
- In `Opc.Ua.Export`: a public `UANodeSet.Import` overload taking a
`NodeSetImportStateFactory`, and a public batch
`LinkParentChildRelationships(context, nodes,
NodeSetImportLinkOptions)`. These replace what would otherwise have been
an `InternalsVisibleTo("Opc.Ua.Server")` on `Opc.Ua.Types`.
- `NodeState.IsExplicitlyDefinedChild(context, child)` and `protected
DetachExplicitlyDefinedChild(child)`, used to tell a generated slot
apart from an ordinary child.
### Typed states, no reflection
The generator emits `{Model}NodeSetImportFactoryProvider` — one factory
per model type/declaration, each calling a concrete constructor for an
*empty* state — and makes the generated node manager implement
`INodeSetImportFactoryProvider` by delegating to it. So the builder
resolves the provider from the manager (`NodeManager as
INodeSetImportFactoryProvider`); `Import` also accepts one directly, and
the generated manager exposes `partial void
AddNodeSetImportFactories(List<INodeSetImportFactory>)` for dependency
models. An imported node of a generated type materializes as its
generated `*State` subclass with no type lookup, which keeps the path
NativeAOT-safe.
Because the factories create empty states, an imported instance carries
exactly the children its document declares.
### Reconciliation
When an imported child lands in a slot the generated parent declares
(same BrowseName), it replaces the generated placeholder: the displaced
node and every descendant the document does not carry over are removed
from the manager, and references to them are retargeted at the
replacement. Replacing a node the same `Configure` pass already wired is
rejected with `BadInvalidState` rather than silently dropping the
wiring, and sealing a builder with an unregistered import batch is
rejected too.
### Also
- `RuntimeNodeSetNodeManager` now uses the same importer: its documents
are one batch, so a node may declare a parent in another document of the
group regardless of parse order, and duplicate NodeIds across documents
are detected while parsing.
- Linking resolves an imported child's reference type from its own
inverse reference (previously always `HasComponent`/`HasProperty`).
- Method import keeps the document's `UserExecutable`; it previously
copied `Executable` into it. View import keeps `EventNotifier`.
## Tests
- `NodeSetImporterTests` (15) — discriminator selection, generic
fallbacks, field/value/reference fidelity against a generic import,
cross-document linking, typed replacement of inherited and argument
slots, deferred linking preserving an application `Handle`, duplicate
NodeIds, factory misuse.
- `NodeSetImportBuilderTests` (15) — staging and lookup before
registration, batch linking, attaching to an existing node, provider
resolution, placeholder replacement with reference retargeting,
configured-placeholder rejection, seal/completion guards.
- `NodeSetImportIntegrationTests` (5) — a generated `[NodeManager]`
overlaying its own model on a running server: typed states through the
generated provider, cross-document linking, placeholder replacement, and
browse through a session.
- `NodeManagerGeneratorTests` (+4) — the emitted provider and the
manager's delegation, including that no factory uses
`Activator.CreateInstance`/reflection.
- `NodeSetAotTests` (+1) — the public state-factory import under
NativeAOT.
Full `Opc.Ua.Server.Tests` (4946), `Opc.Ua.SourceGeneration.Core.Tests`
(3821), `Opc.Ua.SourceGeneration.Tests` (165) and `Opc.Ua.Types.Tests`
pass locally on net10.0.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
1aed93bd03 |
Make the predefined node create lifecycle cancellable (#4437)
# Description
Registration of predefined nodes could not be cancelled:
`CreateAsPredefinedNode` ran the whole create lifecycle without a token,
and a cancellation that landed inside `AddPredefinedNodeAsync` could
still leave the manager half-indexed.
**Cancellable create lifecycle.** Adds a
`NodeState.CreateAsPredefinedNode(ISystemContext, CancellationToken)`
overload. The token is threaded through the internal
`CallOnBeforeCreate` / `CreateInternal` / `CallOnAfterCreate` walk, is
checked between nodes and on each convergence pass, and is passed to
every `OnAfterCreate` override. Cancellation leaves the nodes already
completed in the created state, so a later call resumes only the
remaining ones. The existing single-argument overload is unchanged and
delegates with `default`.
**Shared registration helper.** `CustomNodeManager` and
`AsyncCustomNodeManager` each carried an identical private
`CompleteCreateLifecycleForRegistration`. Both are replaced by one
internal `NodeStateLifecycle.CompleteForRegistration`.
`AddPredefinedNodeAsync` now checks cancellation on entry, propagates it
into lifecycle completion, and checks again before
`IndexPredefinedNode`, so a cancelled registration cannot leave a node
indexed, half-created, or a behaviour replacement partially wired.
_(An earlier revision also added `RemoveExplicitlyDefinedChild`
overrides on `MethodState` and `BaseDataVariableState`. Those were
dropped in
|
||
|
|
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> |
||
|
|
39602bccb1 |
Add node creation to the fluent node manager builder (#4428)
# Description
`INodeManagerBuilder` gains a node **creation** surface alongside its
existing lookup surface, so a `Configure` partial can fill a namespace
without a NodeSet or a ModelDesign.
Twelve new members, implemented as ordinary public members on
`NodeManagerBuilder` (new partial file
`NodeManagerBuilder.Authoring.cs`):
| Member | Creates |
| --- | --- |
| `AddFolder(name, parentId)` | a `FolderState` (`Organizes`) |
| `AddObject(name, parentId, typeDefinitionId)` | a `BaseObjectState` |
| `AddVariable<TValue>(name, parentId)` | a `BaseDataVariableState`
whose `DataType`/`ValueRank` come from `TValue` |
| `AddMethod(name, parentId)` | an executable `MethodState` |
| `Add<TState>(node, parentId)` | an already-constructed state of any
`NodeState` subclass |
| `Add<TState>(factory, parentId)` | a state built by a factory that
receives the resolved parent |
| `AddRoot<TState>(node)` | a root, with its existing references left
alone |
| `TryGetNode(nodeId, out node)` | lookup across
created-but-not-yet-registered nodes and predefined nodes |
Each `Add*` for a browse name takes a `string` (qualified with the
manager's default namespace) or a `QualifiedName` carrying an explicit
nonzero namespace index — eight overloads plus the four above.
```csharp
partial void Configure(INodeManagerBuilder builder)
{
INodeBuilder<FolderState> machines = builder.AddFolder("Machines");
builder.AddVariable<double>("Pressure", machines.Node.NodeId)
.OnRead(() => m_sensor.Pressure);
builder.AddMethod("Reset", machines.Node.NodeId)
.OnCall(ResetAsync);
}
```
Three properties make this usable straight from `Configure`:
- **NodeIds are final before the builder comes back.** Every `Add*` runs
the node — and its whole subtree — through the manager's
`INodeIdFactory` before returning, so `OnRead`/`OnWrite` registrations,
which key off `NodeState.NodeId`, stay valid once the node is
registered.
- **Creation is staged, not immediate.** Created nodes are held until
the manager calls `RegisterAuthoredNodesAsync`. That is what lets a node
name a sibling created moments earlier, and it keeps the builder usable
before the manager has finished building its address space.
- **Custom state types stay typed.** `Add<TState>` returns
`INodeBuilder<TState>`, so a hand-written `NodeState` subclass keeps its
type through the fluent chain.
Staging also makes the builder's own lookups see created nodes: browse
paths, `NodeId`, `TypeDefinitionId` and `DataType` resolution all
consult the staged graph before falling back to the manager's predefined
nodes.
### Registration ordering
A new `protected
FluentNodeManagerBase.RegisterAuthoredNodesAsync(builder, ct)` hands the
staged roots to `AddPredefinedNodeAsync`. It runs **after** the
`Configure` delegates and **before** `CompleteConfigureAsync`, so the
reverse-reference pass sees the new nodes and mirrors their references
to externally owned nodes (typically the ns=0 `Objects` folder) into
`externalReferences`. Three call sites emit it in that position:
- the source-generated `CreateAddressSpaceAsync`
(`NodeManagerTemplates`)
- the hosting `FluentNodeManager` (`FluentNodeManagerFactory`)
- `RuntimeNodeSetNodeManager`, which additionally re-runs
`AddReverseReferencesAsync` because its first pass happens before
`Configure`
A builder that created nothing registers nothing, so the call is safe to
make unconditionally.
The surface is available to **every** fluent host — there is no opt-in
gate and no second builder interface. The generated typed builder
(`FluentBuilderGenerator`) forwards the new members like the rest of
`INodeManagerBuilder`.
### Supporting change
`AsyncCustomNodeManager.PrepareAuthoredNodeIdsForRegistration`
(internal) assigns an id to every node in a subtree that still lacks one
and pulls namespace-0 children into the root's namespace. It differs
from the existing `PrepareInstanceNodeIdsForRegistration`, which only
rebases a subtree that collides with a declaration. It is idempotent.
## Related Issues
- Follow-up tracked in #4429 (adopting this surface in the remaining
hand-rolled node managers, which each need a base-class change first).
- No tracking issue for the API itself yet; happy to open one if
maintainers would like the design recorded as an ADR before review.
## Notes for reviewers
Three judgement calls worth a look:
1. **Always-on rather than gated.** The feature is available to every
fluent host with no enable call. There is no `INodeSource`-style concept
here to withhold it from, so an always-on capability is the same
behaviour with one fewer state flag. Misuse is still rejected: `Add*`
after `Seal()` or after registration throws `BadInvalidState`, a NodeId
in a namespace the manager does not own throws `BadNodeIdInvalid`, and a
parent in one of the manager's *own* namespaces that was never created
throws `BadNodeIdUnknown`.
2. **Detaching from the external-parent proxy goes through
`AddChild`/`RemoveChild`.** The factory overload hands the factory an
identity-only proxy when the parent belongs to another node manager,
then has to drop the parent link the factory established.
`BaseInstanceState.Parent` has an `internal` setter scoped to
`Opc.Ua.Types`, so this pair is the only public route. Commented as such
at the call site. An alternative would be widening that setter or adding
`InternalsVisibleTo` — happy to switch if preferred.
3. **`PrepareNodeIds` does not re-walk per descendant.** The root pass
already covers the subtree, so per-child calls would be no-ops on a
quadratic walk; descendants are only namespace-validated.
One inherited behaviour the new tests pinned down:
`BaseObjectState(parent)` sets `ReferenceTypeId = HasComponent` in its
own constructor, so a node built via the factory overload lands under
the `Objects` folder with `HasComponent`, while the plain `AddObject`
path picks `Organizes`. That asymmetry predates this PR; flagging it in
case it is worth normalizing separately.
`Import(UANodeSet)` is deliberately **not** part of this change — it
belongs to NodeSet import rather than node creation.
## Test results
19 new tests in
`tests/Opc.Ua.Server.Tests/Fluent/NodeManagerBuilderAuthoringTests.cs`
cover NodeId assignment before return, parent-by-NodeId nesting, the
default Objects-folder placement and its inverse reference, custom state
types, both factory paths (authored parent and external-parent proxy),
`AddRoot` reference preservation, `TryGetNode`, id-keyed read handlers
firing on the registered instance, browse-path/NodeId visibility of
created nodes, and every rejection path listed above.
Run against **net10.0** (`-p:CustomTestTarget=net10.0`):
| Suite | Result |
| --- | --- |
| `Opc.Ua.Server.Tests` | 4926 passed, 0 failed, 5 skipped |
| `Opc.Ua.SourceGeneration.Core.Tests` | 3809 passed, 0 failed, 8
skipped |
| `Opc.Ua.SourceGeneration.Tests` | 165 passed, 0 failed |
`UA.slnx` builds with 0 errors and no new warnings.
## Checklist
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation. (New `Creating nodes
from scratch — the Add* surface` section in `docs/NodeManagers.md`, plus
TOC entry.)
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. — **net10.0
only so far** (results above); .NET Framework has not been run locally
yet.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings. — pending first CI run.
- [ ] I have addressed **all** PR feedback received.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
02ea934a31 |
Complete fluent node manager extensibility (#4418)
# Description Complete the fluent node-manager extensibility surface needed by dynamic and externally backed address spaces. This change: - adds virtual node families with cheap NodeId ownership checks, async materialization, per-operation caching, custom browsing, and family-level callbacks; - adds monitored-item pre-creation decisions, complete lifecycle hooks, async batch hooks, stack-managed custom monitored items, and subscription-gated polling; - exposes the startup `ApplicationConfiguration` to generated manager partials through `FluentNodeManagerBase`; - fixes typed builder proxy support, concrete object wrapper types, and fully-qualified generated model composition; - reports unresolved `[NodeManager]` namespace expressions as `MODELGEN035` instead of silently dropping them; - documents the new APIs and compatibility behavior. ## Related Issues - Fixes #4397 - Fixes #4398 - Fixes #4399 - Fixes #4403 - Fixes #4404 - Fixes #4405 ## Validation - `Opc.Ua.Server.Tests` on `net10.0`: 4,886 passed, 5 skipped. - `Opc.Ua.SourceGeneration.Core.Tests` on `net10.0`: 3,807 passed, 8 skipped. - `Opc.Ua.SourceGeneration.Tests` on `net10.0`: 164 passed. - Runtime and generator regression selections passed on `net48`. - Review-feedback validation: 376 fluent tests passed on `net10.0`; 22 focused tests passed on `net48`. - The CI timing regression was repeated five times on `net48` and once on `net10.0`. - All 266 applicable GitHub Actions and Azure Pipelines checks, including CodeQL and coverage, pass on `83c571423`. - AppVeyor reports an immediate zero-duration no-run failure because `appveyor.yml` whitelists only the `appveyor` branch; it did not execute this PR. - `UA.slnx` builds successfully for `net10.0`; affected project builds are warning-free. - The repository-wide `dotnet test UA.slnx` invocation is currently blocked by the solution's mixed VSTest/Microsoft.Testing.Platform configuration under the .NET 10 SDK, so the affected test projects were run individually. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [x] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Copilot-Session: afaa67c9-fc97-4185-a785-edd54a937d95 |
||
|
|
90fc3cca2f |
Complete WoT Binding 1.1 alignment and conformance coverage (#4411)
# Description Completes the UA-.NETStandard implementation of the final OPC UA WoT Binding 1.1 semantics and aligns the WoT Connectivity 1.1 model with xRegistry 0.4.0. The change: - retires `uav:isEvent` and `uav:severity` from current Binding output while preserving unknown members through the generic converter's ordinary residue mechanism; - removes `uav:severity` from the asset Thing Description parser and materializer, so document metadata cannot set occurrence severity; - implements logical EventType definitions, ordered resolution providers, explicit/default selection precedence, materialized member paths, and the built-in `BaseEventType` fallback; - adds deterministic portable identity and browse-path anchoring; - populates bound type declarations instead of creating duplicate siblings, including inherited and closed-content validation; - resolves and validates `uav:externalSchema` without overriding authoritative DataType definitions; - hardens DataType dimensions, native projections, preservation envelopes, URI transmission, projection limits, and delete-policy handling; - synchronizes the xRegistry and WoT Connectivity source-generation inputs; - pins all 27 specification examples, all 97 implementation-owned normative requirements, and all 12 cross-language vector groups to the final specification revision; - adds `samples/WotCon/run-aggregation-demo.ps1` for a verified one-command aggregation client/server round trip and replaces the README topology sketch with Mermaid; - normalizes redirected PowerShell diagnostic presentation in the statement-digest test harness so its semantic assertions are stable on Windows, Linux, and macOS. WoT Connectivity remains model version `1.1`; draft iterations are identified by release labels such as `1.1-draft5`. **Specification dependency:** the evidence assets pin `OPCF-Members/spec-drafts` `main` commit `25db507121b503fd31c8816fbfae67a6822e813b`. The corresponding specification follow-up is `OPCF-Members/spec-drafts#35`. ## Validation - Latest GitHub/Azure CI: 266/266 checks passed. - .NET 10 feedback-focused tests: - WoT Connectivity event/model/traceability: 72 passed - binding capability/traceability: 8 passed - coverage-gate tests: 25 passed - Complete .NET 10 Tools project: 752 passed, 1 intentionally skipped. - .NET Framework 4.8: - WoT Connectivity event/model/traceability: 72 passed - WotCon bindings build: 0 warnings, 0 errors - NativeAOT build: 0 warnings, 0 errors. - WoT aggregation demo: 16 documents uploaded and all 10 mapped values read with Good status; exact processes and isolated PKI stores cleaned up. - Specification traceability: 402 normative statements mapped; all 97 implementation-owned statements have passing test evidence with zero gaps. - Shared conformance vectors: 12/12 groups execute. - Final CI coverage: - project line coverage: 86.83% (floor 70%) - project branch coverage: 76.71% (floor 60%) - patch coverage: 100.00% (2,591/2,591 changed lines) - coverage gate passed ## Related Issues Follow-up to #4375 and alignment with `OPCF-Members/spec-drafts#31` and `OPCF-Members/spec-drafts#35`. No separate tracking issue was opened for this follow-up. ## 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 in the changed paths. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [x] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed all PR feedback currently received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a9f41b1-72f0-4c74-bd7f-26fa531e5738 |
||
|
|
e7542b7f5f |
Fix generated structured defaults and dependency metadata (#4414)
# Description Fix two source-generator paths that lost variable values or metadata: - XML `ExtensionObject` defaults without a `TypeId` now resolve their body through the encodeable factory's registered XML qualified name, decode to the generated structure, and retain the XML encoding identifier. - The existing preview `ModelDependencyV1` payload now carries effective child access and user-access values, minimum sampling interval, historizing state, and serialized default-value XML across referenced assemblies. The dependency validator rehydrates this metadata before inherited instance factories are generated. Explicit non-null extension-object TypeIds remain authoritative, unknown XML bodies remain raw XML, and explicit design files remain authoritative over referenced payloads. ## Related Issues - Fixes #4401 - Fixes #4402 ## Validation - Focused decoder, payload, validator, and Roslyn producer/consumer regressions on `net10.0` and `net48` - `tools/SourceGeneration.slnx` Debug builds on `net10.0` and `net48` - `tools/SourceGeneration.slnx` Release build on `net10.0` - Scoped `UA.slnx` Debug build on `net10.0` ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Copilot-Session: 66e6d342-d4a0-4d53-822b-524982c62f15 |
||
|
|
fd7c88c136 |
Add federated OpenUSD site demo (#4393)
# Description Adds a one-command federated OpenUSD site demo and fixes the composition path that previously produced an empty, overlapping, or poorly framed viewport. - Add `samples/OpenUsd/SiteCompositionServer/run-site-composition-demo.ps1` to build and start the pump, generator, and site servers, wait for every endpoint, publish the connector and viewer side by side, launch the federated viewport, and clean up child processes and artifacts. - Include every fetched root layer in the generated stage and prepare structural component composition before the retained renderer starts. - Continue persisting structural changes while forwarding value-only updates into the live viewport, avoiding renderer topology invalidation. - Fold structured translation, rotation, and scale values into `xformOp:transform` matrices in `UsdFileSink`, including time samples, so multiple pump and generator instances remain distinct when the stage is reopened. - Preserve the latest prior transform components when independently sampled translation, rotation, or scale history adds a new matrix sample. - Reframe the site camera to show all three default pumps and both generator sets. - Move the detailed root sample catalog to `docs/samples.md` and replace it with a concise link to the platform-independent samples and companion sample repository. Validation performed: - Ran the demo script through both its existing-viewer-bundle path and its default side-by-side publish path on isolated ports. - Ran the focused OpenUSD connector and file-sink tests: 30 passed. - Ran the full `Opc.Ua.OpenUsd.Tests` project: 919 passed; the remaining local failure is the existing culture-sensitive decimal formatting assertion on this host's `en-DE` culture. - Ran the focused review-feedback file-sink tests: 19 passed. ## Related Issues - Fixes #4392 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Copilot-Session: 797ea461-49e2-4844-b6a4-54565a9469bd |
||
|
|
c84c168585 |
Merge OpenUSD scene packages (#4384)
# Description Merge the draft OpenUSD Scene Materialization projects into the existing OpenUSD core and server packages: - move the scene companion model, document/value model, and `.usda` conversion APIs into `Opc.Ua.OpenUsd`; - move discovery, materialization, export, and Part 1 interop into `Opc.Ua.OpenUsd.Server`; - rename the CLR APIs to `Opc.Ua.OpenUsd.Scene`, `Opc.Ua.OpenUsd.Scene.Conversion`, and `Opc.Ua.OpenUsd.Server.Scene`; - remove the `Opc.Ua.OpenUsdScene` and `Opc.Ua.OpenUsdScene.Server` projects and NuGet package IDs; - update the connector, the consolidated OpenUSD test project, package READMEs, and developer docs. This is an intentional source- and binary-breaking cleanup for the draft Scene packages. The OPC UA Scene model URI, version, NodeIds, and wire behavior remain unchanged. ## Related Issues - Fixes #4383 ## Validation - `UA.slnx` Release builds completed for `CustomTestTarget=net10.0` and `CustomTestTarget=net48`. - OpenUSD tests passed on net10.0 (915) and net48 (879), excluding the existing host-culture assertion; the isolated assertion passed under invariant globalization. - The generated Release pack contains the expected 75 packages, emits neither retired Scene package, and passes `.azurepipelines\validate-source-generator-packages.ps1`. - The two surviving packages expose the new nested namespaces and contain no retired package dependencies. ## 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
df44b460be |
Complete CreateInstanceOf node lifecycle during registration (#4360)
# Description Generated `CreateInstanceOf<Type>` factories build typed node graphs and assign per-instance NodeIds, but intentionally leave the node create lifecycle open. Hand-written callers commonly registered those graphs without calling `CreateAsPredefinedNode`, so `OnBeforeCreate`, `OnAfterCreate`, and change-mask clearing never ran. This change: - adds `NodeState.IsCreated` and makes lifecycle completion idempotent per node; - completes newly materialized children, including children created by `OnAfterCreate`; - bounds late-child lifecycle convergence and fails explicitly if callbacks keep adding uncreated nodes; - completes uncreated nodes automatically before predefined-node registration and behavior wiring in both async and legacy custom node managers; - assigns/rebases async-node-manager NodeIds before lifecycle callbacks; - emits a null-safe Debug diagnostic when registration performs completion; - documents the generated-factory lifecycle contract and migration behavior; - adds regressions for condition/alarm method binding, finite-state-machine namespaces, Fluent state-node rebasing, behavior replacement, cloning, deletion, late children, non-converging callbacks, and synchronous/async registration. The tracked `src/` and `samples/` call-site audit covered 2,953 C# files. No additional ordering-sensitive call sites require explicit completion beyond the three existing DI/Robotics sites. ## Validation - `dotnet build UA.slnx -c Release -p:CustomTestTarget=net10.0`: passed. - `Opc.Ua.Server.Tests` net10.0: 4,811 passed, 5 skipped. - `NodeStateTests` net10.0 and net48: 213 passed on each. - Node-manager lifecycle and null-logger tests net48: 459 passed. - Source-generation core tests net10.0 and net48: 3,802 passed, 8 skipped on each. - DI, Robotics, ISA95, Positioning, Vision, Vision.Intent, and XRegistry suites passed on net10.0 and net48. - Generated alarm lifecycle regression passed on net10.0 and net48. The solution build reports six pre-existing CA warnings in untouched files. The OpenUSD suite has one pre-existing locale-sensitive failure in `StructuredCartesianCoordinatesAreAccepted` (`1.5` expected versus `1,5` under `en-DE`); all other OpenUSD tests passed on both tested frameworks. ## Related Issues - Fixes #4320 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7ffdc400-0710-4a2e-a9d1-c0e1a9b3b81e |
||
|
|
b97fd8d4b5 |
Fix OPC 10000-21 onboarding interoperability (#4385)
# Description Fix the four blockers that prevented the OPC 10000-21 onboarding registrar and client from working end to end: - generate every `ModelDesign` target independently, including multiple models in one directory, while preserving stable identifier CSV matching and cross-model dependencies; - ship the generated `Opc.Ua.Onboarding` constants, node states, encodeables, and typed clients from `Opc.Ua.Gds.Common`; - correct the standard `EncodedTicket` datatype to derive from `ByteString`; - align the new 2.0 `OnboardingClient` API with the generated `ArrayOf<ByteString>` / `ArrayOf<StatusCode>` contract; - make `BindToTicketStore` fully asynchronous, cancellation-aware, and assign the declared output slot instead of appending a second output; - add generated-model, client fallback, server binding, and in-process end-to-end regressions. The change also adds `samples/Gds/OnboardingRegistrar`, `samples/Gds/OnboardingClient`, and `run-onboarding-demo.ps1`. The demo loads the generated GDS and Onboarding models, restricts ticket administration to an ephemeral `RegistrarAdmin` user, proves anonymous access is denied, and completes a secure register/unregister round trip. The History CI follow-up rejects negative and non-finite processed-history intervals before provider dispatch and enforces the buffered aggregate output cap while calculator results are flushed. This prevents malformed requests from running until the client timeout, exhausting memory, delaying server teardown, and causing later alarm-event tests to fail. ## Related Issues - Fixes #4378 - Unblocks OPCFoundation/UA-.NETStandard-Samples#819 - Tracks the canonical model correction in OPCFoundation/UA-Nodeset#122 ## Validation - Full `net10.0` test projects: - `Opc.Ua.SourceGeneration.Core.Tests`: 3803 passed, 8 skipped - `Opc.Ua.SourceGeneration.Tests`: 143 passed - `Opc.Ua.Gds.Tests`: 1114 passed, 42 skipped - `Opc.Ua.History.Tests` with the CI coverage settings: 506 passed, 27 skipped - `HistorianDispatcher` tests: 88 passed on `net10.0`. - Invalid processed-history interval regressions passed on `net10.0` and `net48`. - Focused onboarding/source-generation regressions passed on `net48`. - `dotnet build UA.slnx -p:CustomTestTarget=net10.0 --no-restore` succeeded; the reported analyzer warnings are pre-existing in unchanged XRegistry/WoT files. - Packed all three GDS packages and verified the packaged `Opc.Ua.Gds.Common` assembly contains the generated Onboarding model surface. - `pwsh samples/Gds/run-onboarding-demo.ps1` verified anonymous denial and the authenticated secure ticket lifecycle. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d2c52c7-7040-4bd6-98de-aaf82ba7846b |
||
|
|
81f200c9d2 |
Align WoT conversion and runtime with Binding 1.1 (#4375)
## Summary Align the generic WoT/NodeSet converter, WoT Connectivity registry, and runtime binding implementation with the OPC UA WoT Binding 1.1 draft. ### Conversion and preservation - fix the standard placeholder ModellingRule NodeIds; - make every produced/restored NodeSet importable by completing required aliases; - support `ua:ComponentOf`, arbitrary companion ReferenceTypes, BrowseNames, InverseNames, symmetric references, and exact direction; - enforce the projection annotation allowlist and deterministic total order; - emit and consume Method argument schemas, EventType data, Alarms & Conditions, event severity, engineering units/ranges, localization, ValueRank, and ArrayDimensions; - preserve unsupported content through JSON residue/native projection rather than silently dropping it; - compare residue conflicts with RFC 8785 canonical equality while retaining byte-exact residue digests; - stamp generated documents with `uav:bindingVersion: "1.1"`. ### Runtime bindings - resolve an EventType Thing Model referenced with `tm:ref`, recursively derive its `data` fields as the baseline selection, and overlay explicit `uav:eventSelectClauses` that also reference EventType TMs; - pre-resolve EventType links asynchronously through the bounded sibling-document resolver before synchronous protocol planning; - retain read compatibility with legacy implementation-specific `uav:eventFields`; - expose nested event `data` matching the TD while retaining the flat transport field index for compatibility; - apply minimum-security constraints through deterministic endpoint selection and fail-closed session verification; - use one Unicode code-point comparer for projection membership and endpoint ties. ### Validation and fixtures - add strict, profile-aware validation while keeping default parsing permissive and residue-preserving; - pin 27 normative examples byte-for-byte to the specification repository, with an offline SHA-256 manifest and regeneration test; - regenerate checked-in WoT documents with the final canonical writer and binding version; - document the generic converter separately from the legacy OPC 10100-1 asset path. ## Notable compatibility changes - converted NodeSets may gain a completed `<Aliases>` table so standard ReferenceType names can be imported; - previously inverted/dangling placeholder ModellingRules are corrected; - generated documents gain binding-version, localization, rank/dimension, and exact ReferenceType metadata; - event data now has the nested shape described by the binding; - the former NodeId-based `uav:typeDefinitionId` clause is removed; event selections now use EventType-TM `tm:ref` links; - `uav:eventFields` remains readable but new documents use `uav:eventSelectClauses`; - unknown well-formed future revision/profile claims produce unsupported warnings rather than strict rejection. ## Generated files Most of the 621-file diff is deterministic regeneration: 517 checked-in WoT documents gain the binding version and final canonical spelling. Regeneration was run twice and is byte-idempotent. ## Validation - `dotnet restore UA.slnx`; - `dotnet build UA.slnx -c Release` across all configured TFMs; - `Opc.Ua.Types.Tests`: 9,215 net10.0; 9,208 net48; 9,208 net472; - `Opc.Ua.WotCon.Bindings.Tests`: 611 each on net10.0, net9.0, and net8.0; - `Opc.Ua.WotCon.Tests`: 1,136 net10.0; - `Opc.Ua.WotCon.Samples.Tests`: 14 net10.0; - NativeAOT test executable: 136/136; - all 27 strict fixture conversion/import checks; - formatting, analyzers, `git diff --check`, and regeneration idempotence. The full net48 run retains one unrelated existing MAX_PATH failure in `FileWotRegistryStoreTests.MultiLevelDirectoryCreationSyncsEveryModifiedParentInOrder`. Solution-wide stress runs also include an intentional 60-minute soak and unrelated locale-sensitive/flaky tests; all affected WoT projects and tests pass. ## Specification Depends on OPCF-Members/spec-drafts#31. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a9f41b1-72f0-4c74-bd7f-26fa531e5738 |
||
|
|
7fe8b30cc3 |
Publish migration plugin marketplace and update NuGet dependencies (#4358)
# Description - Replace the Azure DevOps continuous-preview feed with the GitHub Packages NuGet feed, including its classic PAT `read:packages` requirement, while retaining nuget.org for official public 2.0 preview releases. - Document one-shot MCP execution with `dotnet tool exec` and `dnx`. - Make this repository registerable as the `opcua-dotnet` Copilot marketplace and package `opcua-v20-migration` as an installable plugin. - Bundle the 15 thematic migration documents for offline plugin use and add a CI check that keeps the bundle and marketplace metadata synchronized. - Align the migration analyzer guidance with current master, including the new `UA0030` analyzer while retaining `UA0029` as a runtime-shim-only marker with reserved diagnostic metadata. - Restrict generated collection-shim type discovery to consumer source declarations before the exact `System.<Type>` / `Opc.Ua.<Type>` metadata fallback, with regression coverage for dependency-only metadata. - Update 52 unpinned NuGet package IDs across 60 central entries, plus two schema-test overrides, while preserving documented compatibility and security pins. ## Related Issues - Fixes #4330 ## Validation - `dotnet build UA.slnx -c Release --no-restore`: 0 warnings, 0 errors. - Migration analyzer tests across net472, net48, net8.0, net9.0, and net10.0: 795 passed, 0 failed, 0 skipped. - The migration plugin bundle, manifests, PowerShell, XML, links, and isolated marketplace installation validate successfully. - The direct and transitive NuGet vulnerability audit reports no known vulnerabilities. - A full solution test run was completed before merging current master. Its remaining failures were existing locale, path-length, GPU, shared-state, and flaky integration failures. Persistent cases reproduced on the pre-update commit, while isolated reruns cleared the dependency-relevant Core, GDS, Sessions, Subscriptions, and durable-queue failures. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0ecb0e9-e5ef-4b2f-9922-41a7d42b0372 |
||
|
|
4eccafbe2f |
Remove AAS projects and samples (#4359)
# Description Remove the Asset Administration Shell projects and samples from the mainline repository while preserving the complete implementation and history on the `aas` branch. This change: - removes the `Opc.Ua.Aas`, `Opc.Ua.Aas.Client`, `Opc.Ua.Aas.Server`, and `Opc.Ua.Aas.WoT` projects; - removes the AAS test project, AOT coverage, sample, and documentation; - removes AAS entries from `UA.slnx`, the expected package list, and related build configuration; - removes the now-unused `System.IO.Packaging` package version; and - updates remaining documentation and comments so they no longer advertise removed AAS functionality. The preservation branch `aas` points to the original `master` commit before this removal. ## Related Issues - None. ## Validation - `dotnet restore UA.slnx` - `dotnet build UA.slnx -c Release --no-restore` - Repository-wide sweep for dangling AAS project, package, file, and documentation references The full Release build completed with zero errors. It reported 31 existing analyzer warnings in files unrelated to this change. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. (Not applicable: this removes a feature and its tests.) - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
414957c88a |
Reduce ISubscription: internalize the publish pipeline, delete the dead members (#4349)
# Description Reduces the server subscription API surface per the measurements from issue #4185. ## ISubscription: 42 → 30 members (#4185) A call-site census of the 11 "publish-pipeline protocol" members plus `Publish` (all 68 `src/` projects, samples, tools, fuzzing, tests) found **zero callers outside `Opc.Ua.Server`** and split the members three ways: - **Deleted (4):** `ItemReadyToPublish` and `ItemNotificationsAvailable` were dead — zero callers repo-wide and implementation bodies commented out since before 1.5.378 — the obsolete parameterless `SessionClosed()` had zero callers, and `TransferSessionAsync` (the one-shot direct transfer, which bypassed the reservation protecting a transfer against a concurrently closing source session) lost its last caller once the manager always uses the claim/prepare/commit protocol. - **Internalized (8):** `PublishTimerExpired`, `Acknowledge`, `PublishTimeout`, `SubscriptionTransferred`, `AvailableSequenceNumbersForRetransmission`, `QueueOverflowHandler`, `SessionClosed(ISession)` and `Publish` move to `internal interface ISubscriptionPublishPipeline : ISubscription`, implemented **explicitly** by `Subscription` (the `ISubscriptionMonitoredItemLifecycle` shape) so they are unreachable through the public concrete class too. `Publish` goes with them deliberately — it is the same hazard class (consumes notifications, advances sequence numbers) with the same single production caller. - **Kept public (1):** `ResendData` drops out of the issue's list — its only caller is the `Server_ResendData` UA-method handler in `DiagnosticsNodeManager`, resolving via `TryGetSubscription` like `GetMonitoredItems` does. It is service-facing, not pipeline. ## The pipeline contract is complete: SessionPublishQueue traffics only in it `SessionPublishQueue` becomes `internal sealed` (zero consumers outside the assembly; `SentMessageQueue` already was) and works **solely with `ISubscriptionPublishPipeline`** — no `ISubscription` and no concrete `Subscription` anywhere in the file. To get there, the internal transfer-claim protocol (`TryBeginTransfer`, `PrepareSessionTransferAsync`, `CompleteTransfer`, `AbortTransfer`, `TryRestoreSessionAfterFailedTransfer`) moved from `internal` methods on the concrete class onto the internal interface, implemented explicitly. That completes the contract, so `TransferSubscriptionsAsync` loses its two-tier shape: the ~6 `subscription as Subscription` null-guard branches and the degraded `TransferSessionAsync` fallback for non-`Subscription` implementations are gone — one code path, always the claim/prepare/commit/rollback protocol, and `ISubscription.TransferSessionAsync` is deleted with it. Subscription creation fails with `Bad_InternalError` when a `CreateSubscription` override returns a type not derived from `Subscription`, instead of publishing partially — such implementations were already second-class (transfer required the concrete type before this change). This is the headline behavior change for exotic extenders and is documented prominently in `docs/migrate/2.0.x/sessions-subscriptions.md`. The durable-subscription open question in the issue is answered **no**: `Subscription/Persistence/` never references `ISubscription` (the store deals in `IStoredSubscription` DTOs; restore constructs the concrete type via `Subscription.RestoreAsync` inside the manager), so the internalization cannot break a store implementer. ## #4186: measured, prototyped, and deliberately kept as-is Body inspection showed only 4 of the 10 `ISubscriptionManager` wrappers are pure lookup-and-delegate (`Republish`, `SetTriggering`, `ModifyMonitoredItemsAsync`, `SetMonitoringModeAsync` — one caller each); the other 6 carry logic with no `ISubscription` substitute. A collapse of the four was implemented and then reverted on this branch: it traded ten uniform id-addressed members for a mixed dispatch shape in `StandardServer` without reducing what a reader must understand. **Every subscription service keeps going through `ISubscriptionManager`**; the wrapper bodies now resolve against the pipeline-typed registry. The decision is recorded in `plans/ServerRuntimeAnalysis.md` (open question 5). ## Compatibility treatment Removals are outright, per the documented no-`[Obsolete]`-shim policy for these interfaces (`docs/MigrationGuide.md` — re-adding an interface member would break every implementer; net48/netstandard2.0 have no default interface methods). In its place: - Migration analyzer **UA0030** (pipeline members + `SessionPublishQueue`, distinguishing deleted no-ops from internalized protocol), with tests, following the UA0024 syntax-based pattern. The transfer-claim members were never public in 1.5.378, so they need no migration story. - `docs/MigrationGuide.md#ua0030` section with before/after code; the `docs/migrate/2.0.x/sessions-subscriptions.md` section that told implementers to *add* `SessionClosed(ISession)` is replaced with the derive-from-`Subscription` story; the 2.0.x README symptom table routes the rule. - All 11 removed/internalized members existed in 1.5.378 (verified against `master378`), so each is documented as a 2.0 break. ## Tests Converted, not deleted. `Mock<ISubscription>` setups of pipeline members become `Mock<ISubscriptionPublishPipeline>` — Moq proxies the internal interface via a new `InternalsVisibleTo("DynamicProxyGenAssembly2")` (keyless; the SDK appends `$(PublicKey)` under `SignAssembly`, and signed pipelines do not run tests, matching the existing `Opc.Ua.Redundancy.Kubernetes` precedent). A test-side extension shim forwards the original member names (pipeline + transfer protocol) so the direct call sites on concrete `Subscription` instances compile unchanged. The transfer-race tests' typed reflection probes into `m_abandonedSubscriptions` follow the field's new type. - `Opc.Ua.Server.Tests` (net10.0): 4795 tests, 0 failures - `Opc.Ua.Subscriptions.Tests`, `Opc.Ua.Subscriptions.Durable.Tests`, `Opc.Ua.Subscriptions.Classic.Tests` (net10.0): green - `Opc.Ua.MigrationAnalyzer.Tests`: green incl. the new UA0030 tests - net48 gate: solution build green; subscription-area fixtures green (the known pre-existing local hosting-test environment failures reproduce identically on unmodified master) ## Related Issues - Fixes #4185 - #4186 was measured and prototyped on this branch and deliberately **not** taken (see the section above); the decision is recorded in `plans/ServerRuntimeAnalysis.md`. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. _(net10 suites fully green locally; net48 build green and subscription-area fixtures green — the remaining net48 hosting-test failures on my machine reproduce identically on unmodified master.)_ - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
14fba2d120 |
Fix instance TypeDefinition across dependency ModelDesign boundaries (#4355)
# Description The model source generator crashed with `MODELGEN003` / `NullReferenceException` whenever a `ModelDesign` instantiated an `ObjectType` (or `VariableType`) declared in a **dependency** design file, as soon as that type had children. The sibling of #4332 on the instance-declaration path: #4335 linked dependency *data types*, but an *instance* whose `TypeDefinition` lives in another design still crashed in `NodeStateGenerator` on the type's inherited children. Root cause: dependency design files load with `validateDictionary: false`, so `ValidateInstance` / `ValidateType` never ran on them and their types' children kept `TypeDefinitionNode` / `DataTypeNode` at `null`. When the target instance merged those children into its hierarchy (`BuildInstanceHierarchy` re-walks the dependency type's `Children.Items`), the null references were copied along, and `GetNodeStateClassName` dereferenced them while building the node state factory list. Changes: - A new best-effort `LinkDependencyInstances` pass in `ModelDesignValidator` links dependency-design instances the way `ValidateInstance` / `ValidateType` do for the target model: the `TypeDefinitionNode` / `DataTypeNode` of the types' children and standalone instances (recursively), variable `DataType` / `ValueRank` inheritance from the variable type, `DefaultValue` decoding, `VariableTypeDesign.DataTypeNode` restrictions, and method arguments — including creation of the `InputArguments` / `OutputArguments` argument properties so method children materialise into the target hierarchy exactly like local ones. Resolution is best effort (unresolvable references stay `null`) and NodeSet2 dependencies remain excluded, consistent with `LinkDependencyDataTypes`. - The pass runs alongside `LinkDependencyDataTypes`: for upstream dependencies after all of them are loaded and before the target validates (so the hierarchy merge picks up the links), and for downstream dependencies after they load. - `docs/ModelDependencies.md` documents the instance linking. Tests (all verified to fail with the exact reported NRE stack before the fix): - `CrossModelInstanceTypeDefinitionTests` (Core, 4 tests): the issue's minimal repro end to end; an equivalence test asserting the cross-model instance emits the **identical node-state factory set** as a locally declared type (this is the issue's stated expectation, and it drove the method-argument-property part of the fix); the flipped upstream/downstream ordering; and validator-level assertions that the merged hierarchy carries linked `TypeDefinitionNode` / `DataTypeNode` references, including the data type inherited from a dependency `VariableType`. - `CrossModelInstanceTypeDefinitionGeneratorTests` (Roslyn driver, 2 tests): the issue's csproj repro shape — both `AdditionalFiles` orderings, asserting no diagnostics and that the combined generated output of both models **compiles** (the instance factories reference the dependency model's typed state classes). Verified locally on net10.0: `Opc.Ua.SourceGeneration.Core.Tests` 3800 passed, `Opc.Ua.SourceGeneration.Tests` 141 passed, `Opc.Ua.SourceGeneration.Stack.Tests` 94 passed; 0-warning Release build of `Opc.Ua.SourceGeneration.Core` (all TFMs incl. netstandard2.0); `dotnet format whitespace --verify-no-changes` clean. This removes the last generator blocker for source generating the instance model of the `Workshop/DataTypes` sample (OPCFoundation/UA-.NETStandard-Samples#814): its `ParkingLot` / `DriverOfTheMonth` are instances of types declared in the other design file. ## Related Issues - Fixes #4353 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> |
||
|
|
24ba4855e6 |
Fix [DataType] generator Clone() dropping every inherited field of a derived structure (#4354)
# Description
The `[DataType]` source generator emitted a `Clone()` that created a
fresh instance and copied only the fields declared on the annotated type
itself, so every field inherited from an `IEncodeable` base was silently
dropped. Since the server clones node values while loading an address
space, a derived structure served by a node manager came back out of a
Read with its inherited fields empty, even though encode/decode was
correct.
This PR makes the `[DataType]` path emit the same
`MemberwiseClone`-based shape as the model-based generator:
```csharp
/// <inheritdoc/>
public override object Clone()
{
return (DerivedStruct)this.MemberwiseClone();
}
/// <inheritdoc/>
public new object MemberwiseClone()
{
DerivedStruct clone = (DerivedStruct)base.MemberwiseClone();
// deep copy of the own reference-typed fields
return clone;
}
```
`Clone()` delegates to a generated `public new object MemberwiseClone()`
that starts from `base.MemberwiseClone()` — a shallow copy of the whole
object, inherited fields included — and then deep copies the type's own
reference-typed fields (encodeables, `DataValue`, `Variant`,
`ExtensionObject`). Derived classes hide `MemberwiseClone` and chain
into the base implementation, so deep-copy semantics compose across the
hierarchy, including across assemblies into model-generated base types
(the `BicycleType` scenario that surfaced this). The two generator paths
now agree.
Because the clone now starts from a full shallow copy
(`base.MemberwiseClone()` for classes, `with { }` for records), the
per-field clone emitter only writes the deep-copy assignments; the
redundant per-field plain copies are gone. This also fixes a latent side
bug in the class path: init-only simple fields were silently dropped by
the fresh-instance clone as well.
Records were already correct (`with { }` carries every field) and are
unchanged in shape.
Tests:
- `GenerateClassCloneUsesMemberwiseClone` /
`GenerateDerivedClassCloneCarriesInheritedState` assert the emitted
shape for root and derived classes.
- `DerivedClassCloneKeepsInheritedFields` runs the exact repro from the
issue through the Roslyn driver, emits the assembly, and executes
`Clone()` at runtime to prove the inherited field survives (verified on
net10.0 and net48).
Verified locally: `Opc.Ua.SourceGeneration.Core.Tests` (3798 passed),
`Opc.Ua.SourceGeneration.Tests` (140 passed),
`Opc.Ua.SourceGeneration.Stack.Tests` (94 passed), and
`Opc.Ua.Client.Tests` (2161 passed, exercises the regenerated in-tree
`[DataType]` record types). The in-tree `[DataType]` consumers
(Opc.Ua.Client, both GDS common projects) regenerate and build without
warnings.
## Related Issues
- Fixes #4352
## Checklist
_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
766c5558e8 |
Fix structure subtyping across dependency ModelDesign boundaries (#4335)
# Description The model source generator crashed whenever a `ModelDesign` declared a structure that subtypes a structure from **another** design file (`MODELGEN003` with a `NullReferenceException`), and the cross-assembly `[assembly: ModelDependency]` flow failed even earlier with `The BaseType reference ... is not the expected type: DataTypeDesign`. Structure inheritance across model boundaries — the primary use case of companion-spec layering — could not be source-generated at all. Three root causes, all in the dependency-loading path of the validator: 1. **Dependency design files were never linked.** They load with `validateDictionary: false`, so their `DataTypeDesign`s kept `BasicDataType` at the enum default (`Boolean`) and their fields' `DataTypeNode` stayed `null`. `BinarySchemaGenerator`'s inherited-field merge then dereferenced the null `DataTypeNode`, and `XmlSchemaGenerator` / `DataTypeGenerator` silently emitted `xs:boolean` / `bool` from the default classification. 2. **Dependencies that import the target crashed at load.** Each generation group receives every design file of the compilation as a dependency, including downstream models that import the group's target — those loaded *before* the target they import and failed resolving their `BaseType`. This produced the confusing `AdditionalFiles`-order-dependent failure modes. 3. **The payload flow never reached ModelDesign targets.** `ModelCompilation` supplied `referencedDependencies` only to the NodeSet2 pass, never to the design-file pass, and payload-materialised data types lacked the classification and field links needed to serve as base types of local structures. Changes: - `ModelDesignValidator.ValidateModel` loads dependencies in two phases around the target: upstream models first (topological, referenced models before their referrers), then the target, then models that import the target. - A new `LinkDependencyDataTypes` pass classifies dependency data types (`IsStructure` / `IsEnumeration` / `BasicDataType`) and resolves field `DataTypeNode` / `Parent` best effort. It runs for payload-imported declarations and upstream design-file dependencies before target validation, and for downstream dependencies after they load. NodeSet2 dependencies are excluded — `NodeSetToModelDesign` already links and classifies them with its own semantics. - Payloads for models that are backed by a design file in the compilation are skipped — the explicit file is authoritative (previously, importing both crashed with duplicate symbolic ids). - Payload-materialised data types honor the payload's `IsEnumeration` flag. - The design-file `GenerateCode` pass now receives the referenced assemblies' dependency payloads (`ModelCompilation` + new optional `referencedDependencies` parameter). - `docs/ModelDependencies.md` updated accordingly. Tests (all verified to fail with the exact reported errors before the fix): - `CrossModelStructureSubtypeTests` (Core, 6 tests): the issue's minimal repro end to end — asserting the emitted `.bsd` carries inherited fields with correct types and `SourceType`, the `.xsd` extends the dependency structure (not `xs:boolean`), and the generated C# derives correctly — plus the reversed target/dependency order, both payload flows at generation and validator level, and a three-level chain (target → payload structure → design-file structure). - `CrossModelStructureSubtypeGeneratorTests` (Roslyn driver, 3 tests): both `AdditionalFiles` orderings compiling the generated output, and the full cross-assembly flow (producer assembly generated from the upstream design; consumer resolves the base structure from the reference's payload without re-emitting it). Verified locally on net10.0: `Opc.Ua.SourceGeneration.Core.Tests` 3786 passed, `Opc.Ua.SourceGeneration.Tests` 135 passed, `Opc.Ua.SourceGeneration.Stack.Tests` 94 passed; 0-warning Release builds of `Opc.Ua.SourceGeneration.Core` (all TFMs incl. netstandard2.0), both Roslyn generator variants, Stack and Tester. This unblocks the `[NodeManager]` migration of the DataTypes workshop sample (OPCFoundation/UA-.NETStandard-Samples#757). ## Related Issues - Fixes #4332 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> |
||
|
|
cef83550cb |
OPC UA Vision: a vision-guided bin-picking demo an LLM agent can see and drive (#4235)
# Description Implements **OPC UA — Vision** and uses it to build the demo the companion specifications were drafted for: an eye-in-hand camera watches a bin of parts, a language model *looks at the frame*, decides which part matches the instruction, and commands the robot through Robot Intent — with the OpenUSD viewer showing the arm move and the bin empty out. > **Draft, and stacked.** [#4195](https://github.com/OPCFoundation/UA-.NETStandard/pull/4195) has merged and master is merged in, so what is left on top of `master` is the Vision work plus the unmerged `marcschier/ai-model-management` branch, which the demo needs for the model/dataset/deployment provenance. Review the Vision commits; the AI Model Management work lands with its own PR. ## What is here **The specification, implemented.** `Opc.Ua.Vision` is generated from the spec's own `Opc.Ua.Vision.NodeSet2.xml`, vendored byte-identically (SHA-256 `79C592C6…`, verified against a fresh download and stored LF-preserved like the Robot Intent NodeSet). `Opc.Ua.Vision.Server` builds the §4.2 discovery object, sensors, the coordinate-frame tree, calibrations, media endpoints, inference pipelines and the §9 feedback methods, with rendering and inference behind injectable providers. `Opc.Ua.Vision.Client` is built on the generated ObjectType proxies and composes transforms across the frame tree, which is what turns a detection in camera coordinates into a pose a robot can act on. **Two perception paths behind one contract**, which is exactly what `InferenceLocationEnum` is for. `OnServer` is a deterministic detector derived from the stage's ground truth — no model, no network, no GPU, so CI can exercise the whole loop. `EdgeOffServer` is the agent: it sees the frame over MCP and calls `SubmitDetections`, and the Server publishes results it did not compute. A client reads `DetectionResultType` identically either way. **The agent's eyes.** `tools/Opc.Ua.Mcp.Vision` adds 22 `vision_*` tools, which with the four shared connection tools makes the `vision` profile 26; `vision_get_frame` returns the camera image as an MCP `ImageContentBlock`, so the model looks at pixels rather than reading a description of them. Tool profiles now compose — `--profile vision,robotics` yields 62 tools — so one agent can both see and act without dragging in the full 136-tool catalogue. **The cell.** `samples/Robotics/BinPickingCell` follows the spec's own Robotics-Vision Addendum: the `world → robot_base → flange → gripper_tcp` frame tree with `camera_eih` on the flange, the `HandEye` extrinsics and the `Intrinsics612x512` intrinsics — the addendum's 2448×2048 calibration scaled to the 4×4-binned grid the camera actually delivers. `samples/Robotics/BinPickingClient` hosts the composed MCP catalogue and optionally the viewer. **Documentation.** `docs/Vision.md` (~800 lines) covers the model, hosting, the build context, every topology builder, both perception paths, feedback, the MCP profile and the limitations; linked from `docs/README.md`, with the profile table in `docs/McpServer.md` and READMEs for both samples. ## Evidence, not assertions Each of these was measured on a running system rather than assumed: | Claim | Evidence | |---|---| | The camera sees the bin | 612×512 frame with the bin and all five parts in view — red `(212,47,47)`, green `(61,193,71)`, blue `(54,85,218)`, yellow `(215,196,58)`, orange `(209,119,55)` | | An agent can act on what it sees | Over MCP, `vision_get_frame` returns a 612×512 PNG that decodes from base64 to 1,254,051 bytes, the descriptor beside the detections says 612×512, and all five `BoundingBox2D` centres land **inside** the frame on the pixel colour their own detection claims | | The geometry is right | A detection composed `camera_eih → world` lands on the authored position, residual **0.0000 m** — on both the on-server and off-server paths | | The detector tracks the world | Placing a part drops the next scan from 5 detections to 4, with that part absent | | The robot obeys the agent | Scripted run took command authority and completed a pick and a place, both `Succeeded` | | Tool catalogues are honest | Counted from a running server: vision 26, robotics 40, `vision,robotics` 62, full 136, **zero duplicates** | | Every node is browsable by NodeId | Eight representative NodeIds — both folders, a pipeline, a frame, the sensor, its Calibrations folder, the HandEye calibration, the Media folder — all `Good` | That tool-count check matters because the profile tests collapse names into a `HashSet` and *cannot see* a duplicate registration — so the new tests count registrations, not names. ## Things that were wrong, and are not now Writing the tests found three defects of one kind — code that answered instead of refusing. A zero-norm quaternion was silently rewritten to identity, so a caller with an all-zero orientation got a confident wrong grasp pose. `SubmitCorrection` rewrote a null `ResultId` to empty and called the sink anyway. A bad `StageIdentifier` **killed the host process** inside native `UsdStage.Open`, via an exception .NET cannot catch. All three now refuse loudly and are pinned by tests. **Nodes that existed but could not be reached.** The node manager indexes the Vision root before configurators run, so everything the fluent builder grafted on afterwards — both folders, the pipelines with their Results and Feedback, the frames, the sensors with their optics, calibrations and media endpoints — was never added to the index. Browsing *forward from a parent* worked, because that walks `NodeState.Children` in memory; browsing any of those nodes *by its own NodeId* returned `BadNodeIdUnknown`, which is how an ordinary client and the MCP discovery tools navigate. Registration is now deferred and flushed after each configurator. The first fix covered only the DI path and left the documented non-DI fallback broken in exactly the same way, so `ConfigureVisionAsync` now exists and registers what it builds. That defect survived a green suite because every test browsed children. The new tests assert by NodeId, and were checked against the broken code — with the flush removed, they fail. **Every Method was uncallable.** The Server passed 426 unit tests while not one of its Methods could be invoked over a real session — `RunInference` returned `Bad_TooManyArguments`, and so did every other Method that takes arguments, which is all of them but `StartContinuous` and `Stop`. Three independent faults, each sufficient on its own: the Method nodes were never created (they are Optional children, and the dispatcher guards every attachment with a null check, so it silently attached nothing); nothing declared the arguments, so the stack concluded none were expected; and `MethodDeclarationId` pointed at a synthesised NodeId rather than the NodeSet declaration a client calls with, so the lookup missed the instance Method entirely. The builder now creates the Methods, the `Results` folder and the `Feedback` object when a provider or sink is configured, declares every signature, and names the right declaration. Unit tests could not have caught this: they invoke the handler delegates directly and never go through `MethodState.Call`. It took an end-to-end test over a real session. Relatedly, children created by the generated `CreateOrReplace` helpers carry no `ReferenceTypeId`, so nothing could reach them — fixed once in the build context rather than at the several dozen call sites. **And one where I was wrong.** Earlier in this branch I reversed my own agents' decision and made `SubmitDetections` accept an empty detection set, reasoning that "I looked and the bin is empty" is a correct observation and refusing it forces a correct agent to invent a detection. The reasoning is sound; it is also an argument *against* the specification rather than an implementation of it. §9.5 is normative and explicit — its StatusCode table lists `Bad_InvalidArgument` for "`Detections` empty; or `SubmitCorrection` supplies both or neither corrected array". Worse, **none of §9.5's argument rules were enforced anywhere** — the dispatcher forwarded every submission straight to the feedback sink, so conformance depended on whichever sink a host installed. They are Server obligations, so they now live in the dispatcher, ahead of the sink, proven by strict mocks that would throw if the sink were consulted. The usability problem is real and does not go away by conforming, so it is raised where it can be fixed: [opcua-drafts#70](https://github.com/marcschier/opcua-drafts/issues/70). ## Tracking the updated drafts The Vision, Robot Intent and AI Model Management NodeSets on `opcua-drafts` `main` have moved, partly because of feedback this branch raised. All three are re-vendored byte-identically — verified by comparing git blob hashes against the upstream files rather than by eye — and the implementations follow. **Two statements that were previously inexpressible now have a wire representation.** `SubmitDetections` takes `SceneIsEmpty` and `SubmitCorrection` takes `RetractAll`, so an agent can report "I examined this frame and there is nothing in it" — the terminating condition of the bin-picking task — and can retract a false positive by correcting a result down to nothing. Both are checked in both directions, because the flag is exactly what separates a deliberate empty observation from a lost payload: an empty array without the flag is refused, and the flag with an array attached is refused as two contradictory claims about one frame. Implemented in the dispatcher, the client, the MCP tools the agent drives, and the sample cell. Also absorbed: `LampType` and `LightingMode` became enums (§66), `ProfileName` became `DefaultProfileName` on the endpoint (distinct from the `ProfileName` a caller passes to `GetStreamEndpoint`, which keeps its name), AI `Invoke`/`InvokeAsync` gained `PayloadUri` under the same exactly-one rule, and AI `ListModels` gained a `ContinuationPoint` — because `MaxResults` alone puts everything past the bound permanently out of reach. **OpenUSD is on `0.9.0-alpha`**, verified on the running cell rather than by restore alone: D3D12 backend, 612×512 frame, the bin and all five parts in view and detected — the same result `0.8.0-alpha` produced. The one local workaround that survives is `VisionMethodArguments`, and its reason has changed rather than gone away: the builder materialises these Methods as Optional children through `CreateOrReplace`, which constructs the state object directly and never runs the generated factory that now carries the signatures. Checked rather than assumed — removing the declarations fails five of the eight `VisionMethodSurfaceTests`. ## Review feedback Three threads, all addressed in `0fe2803a3`: - **The AI Model Management work is now a library family, not a sample.** `src/Opc.Ua.AI` (model), `.Inference` (`IInferenceBackend` and backends), `.Server` (node manager) and `.Client` (discovery, reads, calls, artefact transfer), in the same shape as the Robotics and Vision families; the samples become `samples/AI/ModelManagementServer` and `ModelManagementClient`. The client library needed real work rather than relocation: its browsing surface was `private` inside the sample's scenario runner, so moving the file alone would have produced a library nobody could call. - **No cloud-vendor SDK is referenced anywhere.** Inference goes through `Microsoft.Extensions.AI` — an `IChatClient` is implemented by hosted services and on-device runtimes alike, which is exactly the property clause 8.1 asserts. `Azure.Identity` is gone: workload identity is not an Azure feature, every platform projects a token into a file and each SDK reads that file behind its own API, so the resolver reads it directly. The comment claiming `Microsoft.Extensions.AI` would force a `System.Text.Json` bump was stale — it pins 10.0.10, which this repository already centralises on. - **`BinPickingCell` and `BinPickingClient` moved into `samples/Robotics/`**, which is where they belonged: the cell was already linking the arm, its kinematics and both USD assets out of `IntentEnabledRobot`. ## Upstream Work that belonged elsewhere went elsewhere rather than being worked around locally: - **`openusd-dotnet` [#13](https://github.com/marcschier/openusd-dotnet/pull/13)** — merged and shipped in `0.8.0-alpha`. `SilkFrameCapture.Capture` rendered only the first capture of a session and silently returned a blank frame for every one after it. Also filed [#14](https://github.com/marcschier/openusd-dotnet/issues/14) (no camera-by-prim API) and [#15](https://github.com/marcschier/openusd-dotnet/issues/15) (skipped tests reported as passed — which fooled me for several runs). Both are fixed; this branch takes `0.8.0-alpha` and deletes two workarounds, including 143 lines of hand-rolled projection maths. - **`opcua-drafts` [#66](https://github.com/marcschier/opcua-drafts/issues/66)–[#69](https://github.com/marcschier/opcua-drafts/issues/69)** — a close read of the spec produced 22 candidate ambiguities; 12 were discarded as misreadings with clause citations and the remaining 10 filed. One is concrete: five members exist in the NodeId CSV that no clause documents. - **`opcua-drafts` [#70](https://github.com/marcschier/opcua-drafts/issues/70)** — §9.5 could not express an empty observation or a false-positive retraction. **Adopted**, with the explicit-flag resolution argued for: `SubmitDetections` gained `SceneIsEmpty`, `SubmitCorrection` gained `RetractAll`, and the corrected-array rule relaxed from *exactly one* to *at most one*. - **`opcua-drafts` [#71](https://github.com/marcschier/opcua-drafts/issues/71)** — the Vision NodeSet browse-named all 13 `InputArguments` / `OutputArguments` Properties in the Vision namespace rather than the base namespace, so a stack could not find them and every Method in the specification was uncallable. **Fixed**; this branch now tracks the corrected artefact. ## The defect underneath it all The end-to-end suite was removed earlier in this branch because nine of its fourteen tests failed on one symptom: a client could not open a pipeline's `Feedback` object or enumerate its `Results`, even though browsing the same parent listed both. Restoring it meant explaining that, and the explanation was not in Vision. `RelativePathElement` initialised **`IsInverse` to `true`**. A caller that sets a `ReferenceTypeId` and a `TargetName` — which reads as *"the child reached by this Reference"* — was therefore asking the Server for the **inverse** Reference, and got `Bad_NoMatch` with nothing in the request to suggest why. `ObjectTypeClient.ResolveChildNodeIdAsync` is such a caller, and it is what **every source-generated Optional-child accessor in the stack** is built on — the accessor whose own documentation cites `AlarmConditionTypeClient.GetShelvingStateAsync` as an example. Vision is merely where it surfaced, because Vision puts two Optional children on the path an off-Server agent must take. The sibling helper in `StateMachineTypeClientExtensions` already set `IsInverse = false` by hand, which is the shape of a trap rather than a default. The default is now forward, `ResolveChildNodeIdAsync` says so explicitly, and the existing test that had recorded `true` as *expected* now pins `false` with the reasoning. Of the forty `RelativePathElement` construction sites in the tree, that accessor was **the only one relying on the default at all** — every deliberate caller already set it. Two further defects were found underneath it, both real and both fixed: - The generated `CreateOrReplace` helpers construct a child state object directly and leave **`TypeDefinitionId`** unset as well as `ReferenceTypeId`. A child referenced by nothing cannot be browsed; one with no type definition is a malformed Object that a client filtering by type silently skips — which is exactly what `EnumerateResultsAsync` does. Normalisation moved to `VisionNodeManager`, so it also covers the case the builder cannot see: a result published **at runtime** by an inference provider, long after the address space was built. That path produced results a client could list but not read. - `Opc.Ua.AI` shipped without an `AssemblyInfo`, so the solution built with six `CA1014` warnings. I had initially "fixed" this in the Vision client with Browse-based fallbacks, on the strength of a probe that appeared to show `TranslateBrowsePathsToNodeIds` failing even for `Server → ServerStatus`. That probe was wrong: it used an unqualified `ReferenceTypeIds`, which inside a `Opc.Ua.Vision.*` namespace binds to *Vision's* `ReferenceTypeIds` and yields a nonsense NodeId. Chasing that to the end is what turned up the real cause. **The fallbacks are gone** — they would have hidden a stack-wide bug behind a Vision-shaped workaround, and one of them matched on browse name while ignoring the namespace index. `tests/Opc.Ua.Vision.Intent.Tests` is restored and back in `UA.slnx`, passing **14/14** on its own merit rather than through a client-side workaround. A registration test now pins the pipeline's `Feedback` and `Results` children, which nothing covered before. ## Running it, and what that found The demo's premise is that a language model **looks at the bin and reasons about what it sees**. Running it end to end showed it could not, for reasons no test covered, because every one of them lived between components. **The sample did not compile.** Two `CS8604` nullable errors with `TreatWarningsAsErrors`, so neither the cell server nor the demo could be started at all. Worth recording that my first explanation was wrong: CI *does* build this project - the Linux leg's `net10.0` pass compiles it, log-confirmed - so this is not a coverage gap in the workflow, and no workflow change was made. **`Pick` and `Place` never moved the arm.** They were a delay plus a gripper action, so the operations reported `Succeeded` while nothing travelled. They now solve IK once and interpolate in joint space. **And the parts never moved**: the arm swung but the bin stayed full, because nothing tied the held object to the world state. The connector now creates 11 monitored items where it created 6, and a scripted run delivers `SeqNo=151` across 10 messages where it delivered `SeqNo=3` across 1. **The frame was smuggled through a string field.** The media provider put a base64 *data URI* of the PNG into `VisionImageReferenceDataType.Uri` - a field meant to be a *reference*, while the bytes were already returned in the inline `ByteString`. Every response therefore shipped the image twice, and a 1,311,400-byte PNG became a 1,748,536-character string against a `MaxStringLength` of 65,535: **26.7x over**. That is why raising `MaxByteStringLength`, `MaxArrayLength` and `MaxMessageSize` never helped - none of them govern a `String`. Separately the MCP tool assigned raw bytes where `ImageContentBlock` requires base64, so the wire carried `"data":"�PNG..."` - every non-UTF-8 byte replaced by U+FFFD, destroying the image and inflating the response to 6.8 MB. **The camera was aimed at nothing.** The arm's home pose, the flange scan pose the Vision model declared and the joint angles in the USD stage were three independent claims about where the camera was, and they disagreed - the declared flange orientation pointed the view 86 degrees away from the projection camera. Solving them as one thing exposed two further constraints that are easy to miss: the camera has to sit off the tool axis or it photographs its own gripper, and the IK branch has to be elbow-back or a link parks under the camera and fills the frame with the arm's own upper arm. A third was a regression this branch introduced and `Opc.Ua.Robotics.Tests` caught - aiming a straight-down camera from a point on the base's own X-Z plane puts the wrist on the J4/J6 singularity, so every motion away from home fails to solve. A 15-degree camera roll, free because the camera still looks straight down, gets 25 degrees clear, which is the margin the poses before this branch had. **The frame and the detections were different images.** The sensor declared 2448x2048, the clip endpoint 1280x1024, and the renderer produced 640x512 - not even the same aspect ratio. The ground-truth detector projects through the declared intrinsics, so its boxes lived in 2448x2048 space while an agent was handed a 640x512 picture: *"pick the red cube you can see"* arrived with coordinates that pointed off the image. Everything now agrees at 612x512, an exact 4x4 bin of the native sensor with the intrinsics scaled to match; the native size stays in the model and serial number, where it describes the hardware rather than the image. **`LatestClip` was created and never written**, so it reported `Bad_NoDataAvailable` for the life of the Server and a consumer following the model - read the published frame, call the method only if there is none - never got a frame. The dispatcher now publishes a clip it has just encoded, for every Vision server rather than this sample alone. One genuine API gap turned up on the way: `OpcUaServerOptions` exposes `MaxByteStringLength`, `MaxArrayLength` and `MaxMessageSize` and **no `MaxStringLength`**, so a Server that legitimately needs a long string could not raise the limit through the hosting API at all. Added, with tests. It is not the fix for anything above - that was a design bug - but the missing knob was real. ## Not finished - **Two projects sit below the repository's 80% line coverage.** `Opc.Ua.Vision.OpenUsd` is at 71.1% — what is left only executes with a real OpenUSD stage and plugin tree, which is not on every build agent. `Opc.Ua.AI.Inference` is now at **86.7%**, up from 12.9%: the code this branch *wrote* is covered by 25 tests, and `RestChatCompletionsBackend` — previously the whole of the shortfall — is covered by 18 more against a stubbed `HttpMessageHandler`, exercising success, refusal, throttling, retry-after, timeout and cancellation without a network. Everything else clears the bar — `Opc.Ua.Vision.Server` 92.1%, `Opc.Ua.AI` 91.4%, `.Vision.Client` 88.1%, `Opc.Ua.Vision` 85.7%, `Opc.Ua.AI.Server` 80.1%, all measured with coverlet. The CI coverage gate reports itself as advisory and non-blocking. - **`SamplesCollected` is not counted here, and `docs/Vision.md` now says so.** Section 9.4 requires a Server to count a negative example (`SceneIsEmpty` / `RetractAll` carrying a `GroundTruthLabel`) exactly as it counts one carrying geometry. `VisionNodeManager` does not, and structurally cannot: `SamplesCollected` is a property of `LearningJobType`, which the *AI Model Management* companion defines, and Vision reaches the job through a `NodeId` value rather than a Reference **precisely so this model takes no dependency on the model that defines it**. What the Server does guarantee is that the negative example survives the hop intact — both flags are carried verbatim to `IVisionFeedbackSink` — so a host that binds a learning job has everything it needs to satisfy the clause on the counter it owns. Stated in the limitations rather than left implied. ## Related Issues Was stacked on #4195, now merged. No tracking issue yet for the Vision implementation itself — happy to open one if you would like the design recorded as an ADR before this goes further. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. — *Types 8562, Server 4072, Client 2123, InformationModel 1063, Robotics 540, Tools 466, Vision 442, AI 74 and the restored Vision+Intent end-to-end suite 14/14, all green on net10.0. Only `.OpenUsd` (71.1%) is below the 80% bar — see Not finished.* - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. — *`dotnet build UA.slnx -c Release`: 0 errors, 0 warnings.* - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. — *net48: Types 8555, Client 2126, Vision 385 green. Two net48 Server tests fail (`CreateUserManagementSeamBindsTheModelToTheServerAsync`, and `DurableDataValueQueueVerifyReferenceBatchingAsync` only under parallel load); both were confirmed **pre-existing** by re-running them with the `IsInverse` change reverted, which fails identically.* - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. — *The net48 build legs failed because the restored end-to-end project used `ValueTask.FromResult` / `string.Create`, which do not exist on .NET Framework; fixed in `fb57522da` and verified building on every TFM. `DisposeDrainsHeldConnectionCallbackBeforeListenerDisposal` failed once on the Ubuntu Client leg — a 13 ms timing-sensitive reverse-connect disposal test that passes locally 2123/2123 and touches no browse paths.* - [ ] I have addressed **all** PR feedback received. ## Typed Vision-guided Robotics MCP workflows The branch now also makes the demo workflow first-class in the Full MCP profile: - replaces stringified Robotics intent and mission inputs with typed request schemas and exact name-or-NodeId selectors; - adds bounded operation/mission paging, stable step-to-operation identity, retained mission discovery, and `robotics_wait_mission`; - makes `vision_run_inference` return bounded typed detection, inspection, or segmentation summaries in one call; - adds same-session `robotics_vision_pick` to select a detection and submit Pick or Pick/Place work without implicitly taking authority, retrying, cancelling, or waiting; and - starts Publish workers for classic subscriptions so an independently launched OpenUSD viewer receives live robot and part updates. The migration guide documents the intentionally breaking MCP request-schema changes. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b Copilot-Session: 99d1ab42-f83a-41ab-bf3e-85bf81fadace Copilot-Session: bcbc8ac9-5abd-42a8-8bac-e10533790e2e |
||
|
|
e61ad4b306 |
Fully support sub-state machines in the fluent state-machine builder (#4322)
# Description Part 16 §B.3 places `HasSubStateMachine` on the parent **state** node, but the fluent builder attached it to the state-machine **root** because `FluentFiniteStateMachineState` never materialized state nodes at all. Investigating #3821 showed the gap was wider than the reference placement: 1. **No state nodes existed.** The NodeId written into `CurrentState/Id` — `new NodeId(stateId, ElementNamespaceIndex)` — was **dangling**: no node with that id was in the address space. 2. **`AvailableStates` was never populated**, so `GetAvailableStatesAsync` returned empty against a fluent-built server. 3. **The whole client discovery chain was therefore dead.** `ObserveEffectiveStateAsync` enumerates `AvailableStates` and browses `HasSubStateMachine` from each state node — it found nothing. The client side was already spec-correct; only the server deviated. 4. **`HasSubStateMachine` is non-hierarchical** (a subtype of `NonHierarchicalReferences`, `i=32`). Using it as the sub-SM's `BaseInstanceState.ReferenceTypeId` left the sub-SM unreachable from a hierarchical browse of the machine. ## What changed **Element nodes are materialized.** Definition-mode machines now create one `StateType` node per declared state and one `TransitionType` node per declared transition — each a `HasComponent` child carrying `StateNumber` / `TransitionNumber`, with the §B.4 `FromState` / `ToState` / `HasEffect` references and `HasCause` added by `WithCause`. `AvailableStates`, `AvailableTransitions` and `LastTransition` are populated, so `CurrentState/Id` and `LastTransition/Id` resolve to real, browsable nodes. NodeIds are per-instance, so several machines built from one definition coexist. **`HasSubStateMachine` moved to the state node**, with the inverse `SubStateMachineOf`, and the sub-SM is attached as a `HasComponent` child (fixing 4). **Core gains element-id mapping hooks.** `GetCurrentStateId` / `UpdateStateVariable` / `UpdateTransitionVariable` hard-coded the numeric form and were non-virtual, so a subclass could not redirect them. They now route through `public virtual` `GetStateNodeId` / `GetStateId` / `GetTransitionNodeId` / `GetTransitionId`, whose defaults reproduce the previous `(elementId, ElementNamespaceIndex)` convention **exactly** — every stack-shipped and generator-emitted machine is unaffected, and there is a regression test asserting that. **Generated machines report their model namespace.** The generator now emits an `ElementNamespaceUri` override on `FiniteStateMachineType` subtypes, taken from the nearest type that actually *declares* state children — a subtype that only adds behaviour inherits its elements, and their namespace, from its base. **Ordering and validation fixes.** `WithSubStateMachine` and `WithInitialState` are now order-independent (previously, attaching a sub-SM before setting the initial state left the child suspended until the parent left and re-entered the state — silently). `WithSubStateMachine` validates `parentStateId`, and the builder rejects duplicate element browse names, since element NodeIds are now derived from them. ## A defect this surfaced Code that assembles nodes with the generated `CreateInstanceOf<Type>` factories never calls `NodeState.CreateAsPredefinedNode`, so `OnBeforeCreate` / `OnAfterCreate` are skipped for the entire subtree — and `OnAfterCreate` is the only place a state machine resolves its element namespace. It is also where `ConditionState` binds its `Enable` / `Disable` / `AddComment` handlers and `AlarmConditionState` binds the shelving methods, so the impact is broader than state machines. This PR fixes only the two call sites it depends on — DI's `SoftwareUpdateFacetWiring` and Robotics' `RoboticsOperationBuilder`. **The repository-wide audit is deliberately left out of scope and tracked in #4320** (24 files call these factories; none completed the lifecycle). The Robotics regression had no test coverage at all, so an assertion on `CurrentState/Id` was added and verified to fail without the fix. ## Breaking changes - Sub-state machines are **no longer browsable from the state-machine root**. Callers passing a machine's `ObjectId` to `GetSubStateMachineAsync` must pass a state NodeId instead — from `GetAvailableStatesAsync` or a snapshot's `CurrentStateId`. - `CurrentState/Id` for fluent machines changes from the numeric form to the materialized node's id. Code reading a numeric id out of it should call `FiniteStateMachineState.GetStateId` rather than parsing the identifier. - Generated machines qualify `CurrentState/Id` / `LastTransition/Id` with their companion-spec namespace instead of the OPC UA one. Documented in `docs/WhatsNewIn2.0.md` and `docs/StateMachines.md`. ## Notes for reviewers - Adding `virtual` to the `FiniteStateMachineState` members is a binary (not source) breaking change — assumed acceptable on the 2.0 line, but worth confirming against the compatibility policy. - Address-space cost is one node plus one property per declared state and transition, per machine instance; called out in the docs for servers instantiating many machines. - `AvailableTransitions` is modelled as a `HasComponent` `BaseDataVariable` (matching the standard NodeSet and what the client's browse path expects), not a Property. ## Related Issues - Fixes #3821 - Surfaced #4320 (node create lifecycle audit — not addressed here) ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ### Test runs (net10.0) | Suite | Result | |---|---| | `Opc.Ua.Server.Tests` (full) | 4734 passed, 5 skipped | | `Opc.Ua.SourceGeneration.Core.Tests` | 3780 passed | | `Opc.Ua.SourceGeneration.Tests` | 132 passed | | `Opc.Ua.SourceGeneration.Stack.Tests` | 94 passed | | `Opc.Ua.Types.Tests` | 8763 passed | | `Opc.Ua.Robotics.Tests` | 540 passed | | `Opc.Ua.Robotics.Intent.Tests` | 22 passed | | `Opc.Ua.Di.Tests` | 424 passed | | `Opc.Ua.ISA95.Tests` | 136 passed | | `Opc.Ua.Aas.Tests` | 670 passed | | `Opc.Ua.Positioning.Tests` | 69 passed | | `Opc.Ua.Client.Tests` (StateMachines) | 36 passed | Only net10.0 was run locally; the .NET Framework leg is left to CI, hence the unchecked box above. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> |
||
|
|
41db5aca60 |
Fluent: publish Configure-created nodes' references on foreign-manager nodes (Objects folder placement) (#4331)
# Description A node created inside the fluent `Configure` hook (source-generated `[NodeManager]` partial, or the hosting `AddNodeManager(uri, build)` route) had no way to appear below the Objects folder — or below any node owned by another node manager. `AsyncCustomNodeManager.AddReverseReferencesAsync` mirrors inverse references into the `externalReferences` dictionary **before** the `Configure` partials run, and the fluent surface never sees the dictionary, so a configure-created node with an inverse `Organizes` reference to `ObjectsFolder` existed and simulated but never showed up under Objects. This implements the three coordinated pieces proposed in #4329, plus the companion namespace gap: **1. Idempotent mirroring pass** (independently worthwhile hardening) - `AddExternalReference` in `AsyncCustomNodeManager` and `CustomNodeManager2` now has if-missing semantics (linear scan of the per-source list). - `TypeTable.AddEncoding` no longer appends a duplicate encoding id; re-registering the same encoding is a succeeding no-op. (`AddRootNotifierAsync` and the node-side `AddReferenceIfMissing` branch were already idempotent.) **2. Re-run the pass after `Configure`** - New protected `FluentNodeManagerBase.CompleteConfigureAsync(externalReferences, ct)` wraps `AddReverseReferencesAsync` so hand-written managers using `CreateFluentBuilder` benefit too. - The generator-emitted `CreateAddressSpaceAsync` and the hosting `FluentNodeManager` invoke it once between the `Configure` callbacks and `Seal()`. Timing is safe: the master distributes `externalReferences` only after every manager's `CreateAddressSpaceAsync` returns. Side benefit: inverse `HasNotifier` references on configure-created event sources get root-notifier registration for free. - The hosting `FluentNodeManager`'s hand-rolled `externalReferences` workaround for its root folder is deleted — the root's inverse `Organizes` reference now flows through the shared pass (mirrored exactly once, covered by a test). **3. Discoverable fluent sugar** - `OrganizedBy(parentId)` / `UnderObjectsFolder()` on `INodeBuilder` write the inverse `Organizes` reference; with piece 2, placement is just "write the inverse reference". - A parentless `CreateInstance<TState>` on `INodeManagerBuilder` takes a constructor-style factory (`p => new BoilerState(p)`, keeping the surface reflection-free and AOT-safe), materializes the subtree from the type model via `NodeState.Create`, rebases all NodeIds through the manager's `INodeIdFactory` (`AssignInstanceNodeId`/`AssignInstanceChildNodeIds` — the same pair the generated factories use, so declaration-id children cannot collide with the type model), and registers via `AddPredefinedNodeSynchronously`. The Boiler #2 shape becomes fully fluent: ```csharp builder.CreateInstance( new QualifiedName("Boiler #2", NamespaceIndexes[1]), p => new BoilerState(p)) .Configure(n => n.UnderObjectsFolder()); ``` **Companion gap, same theme:** `[NodeManager]` grows `AdditionalNamespaceUris`, flowing through attribute discovery → binding → generator so the generated constructor reports a second (instance) namespace at construction and the generated factory advertises it in `NamespacesUris` — today `SetNamespaces` after construction is not enough because `MasterNodeManager` builds its namespace routing from what the manager reported when it was built. **Scope boundary** (as discussed in the issue): startup-time configuration only; for nodes created after startup the correct primitive remains `IMasterNodeManager.AddReferencesAsync`. **Reviewer notes** - The generated-code change was validated against the real pipeline: a forced fresh compile of `Opc.Ua.Server.Tests` (whose CoverageNodeSet managers are `[NodeManager]`-generated) emits `await CompleteConfigureAsync(externalReferences, cancellationToken)` between `Configure` and `Seal()` and compiles against the real base class. - The distribution consumer (`AddReferencesAsync`) already used `AddReferenceIfMissing`, so the whole chain is defense-in-depth idempotent. - 15 new tests: double-run idempotence, second-pass pickup of late-registered nodes, the new reference sugar, root-instance creation incl. child-id rebasing and reference remapping, hosting end-to-end Objects-folder placement, emitted-sequence and `AdditionalNamespaceUris` generator tests, and a full Roslyn-pipeline attribute test. Docs updated in `docs/NodeManagers.md` (new "Creating nodes under other managers' nodes" section, attribute docs, fluent-surface sections). - Verified locally: all touched projects build with 0 warnings; full `Opc.Ua.Server.Tests` (4,725 passed / 0 failed / 5 skipped, net9.0), `TypeTableTests` (113), `Opc.Ua.SourceGeneration.Core.Tests` (41), `Opc.Ua.SourceGeneration.Tests` (132), and the Quickstarts.Servers sample all pass. ## Related Issues - Fixes #4329 - Motivating case: OPCFoundation/UA-.NETStandard-Samples#756 / OPCFoundation/UA-.NETStandard-Samples#795 (Workshop/Boiler migration) ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
24399f1808 |
Fix source generator MODELGEN003 diagnostic dropping message placeholders (#4310)
## Problem
`LoggerAdapter.Log<TState>` in
`Tools/Opc.Ua.SourceGeneration/SourceGeneratorTelemetry.cs` reported an
**already fully-formatted** logger `message` against a looked-up
`DiagnosticDescriptor` whose `messageFormat` may declare more positional
placeholders than the single formatted string carries.
For `EventId == 3` the descriptor is `SourceGenerator.Exception`
(`MODELGEN003`) with:
```
messageFormat = "Exception during model generation '{0}': {1}"
```
That format has **two** placeholders but only **one** argument
(`message`) was supplied. Roslyn's `Diagnostic.GetMessage()` runs
`string.Format(format, args)`, which throws `FormatException` because
`{1}` has no argument; Roslyn's internal catch then returns the **raw,
unsubstituted template**. The user therefore saw the literal text
`Exception during model generation '{0}': {1}` with the placeholders
unreplaced, masking the real error.
## Fix
Route the pre-formatted message through a new
`CreateFormattedDiagnostic` helper that reports it via a **passthrough
descriptor whose `messageFormat` is exactly `"{0}"`**, while preserving
the original descriptor's `Id`, `Title`, `Category`, `DefaultSeverity`,
`IsEnabledByDefault`, `HelpLinkUri` and custom tags:
```csharp
public static Diagnostic CreateFormattedDiagnostic(
DiagnosticDescriptor descriptor,
string message)
{
var passthrough = new DiagnosticDescriptor(
descriptor.Id,
descriptor.Title,
"{0}",
descriptor.Category,
descriptor.DefaultSeverity,
descriptor.IsEnabledByDefault,
helpLinkUri: descriptor.HelpLinkUri,
customTags: descriptor.CustomTags.ToArray());
return Diagnostic.Create(passthrough, Location.None, message);
}
```
`LoggerAdapter.Log` now calls this helper instead of
`Diagnostic.Create(descriptor, Location.None, message)`. This renders
the message verbatim under the correct diagnostic id regardless of the
descriptor's placeholder arity, and is safe even when the message itself
contains `{`/`}` characters (the format string is only `"{0}"`).
> Note: the public `descriptor.CustomTags` (`IEnumerable<string>`) +
`.ToArray()` is used because Roslyn's `ImmutableCustomTags` property is
`internal`.
### Why the helper lives in `SourceGeneratorHelpers.cs` (not
`SourceGenerator.cs`)
`SourceGeneratorTelemetry.cs` and `SourceGeneratorHelpers.cs` are shared
(glob-linked) into **both** the model generator
(`Opc.Ua.SourceGeneration`) and the stack generator
(`Opc.Ua.SourceGeneration.Stack`). The stack generator has its **own**
`SourceGenerator` partial class and does **not** compile the model
generator's `SourceGenerator.cs`. Placing the helper in
`SourceGenerator.cs` broke the stack build with `CS0117:
'SourceGenerator' does not contain a definition for
'CreateFormattedDiagnostic'`. `SourceGeneratorHelpers.cs` is the shared
partial (where `Guard` also lives), so the helper is available to both
generators.
### Guard path untouched
The other emission path, `SourceGenerator.Guard(...)` in
`SourceGeneratorHelpers.cs`, uses the same `Exception` descriptor but
correctly passes **two** args (`ex.Message`, `ex.ToString()`), so that
path was already fine and is **not** changed.
## Tests
New
`tests/Opc.Ua.SourceGeneration.Tests/SourceGeneratorTelemetryTests.cs`
(4 tests):
1. **`DirectReportAgainstMultiPlaceholderDescriptorLosesMessage`** —
documents the pre-fix behaviour: the raw `Diagnostic.Create(Exception,
..., "boom")` render drops `"boom"` and leaks `{1}`.
2. **`CreateFormattedDiagnosticRendersMessageVerbatim`** — the fix
renders `"boom"` exactly, with no `{0}`/`{1}`.
3. **`CreateFormattedDiagnosticPreservesDescriptorMetadata`** — id
(`MODELGEN003`), severity, category, help link, IsEnabledByDefault and
custom tags are preserved.
4. **`CreateFormattedDiagnosticIsSafeForBracesInMessage`** — a message
containing `{`/`}` is emitted verbatim.
## Validation
- `tools/Opc.Ua.SourceGeneration` (and the stack variant) build with **0
warnings / 0 errors**.
- Full `Opc.Ua.SourceGeneration.Tests` suite on net10.0: **132/132
passing**, including the 4 new tests.
## Files changed
- `tools/Opc.Ua.SourceGeneration/SourceGeneratorHelpers.cs` — added
`using System.Linq;` + `CreateFormattedDiagnostic` helper.
- `tools/Opc.Ua.SourceGeneration/SourceGeneratorTelemetry.cs` —
`LoggerAdapter.Log` calls the helper.
- `tests/Opc.Ua.SourceGeneration.Tests/SourceGeneratorTelemetryTests.cs`
— new regression tests.
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
||
|
|
06ae192084 |
Fix CI failures tracked by #4299 (#4308)
## Summary - stabilize the History, authentication lockout, Robotics, redundancy, and DI tests reported in #4301-#4305 - fix the closed-transport buffered-write completion deadlock that hung `ConnectMultipleSessionsAsync` - add macOS crash diagnostics for #4306 and retain evidence for root-cause analysis - run macOS tests and NativeAOT in GitHub Actions pull requests, using supported Intel and ARM64 runners, while keeping every Azure macOS definition disabled by default - move all remaining source-level `InternalsVisibleTo` declarations into project files so signed OpenUSD builds receive the repository public key ## Local validation - MCP PubSub: 29 passed - History focused tests: 3 passed - Sessions lockout: 3 passed - Sessions multiple-session regression: 3 passed - deterministic binary client channel tests: 22 passed - Robotics runtime: 34 passed - Redundancy focused tests: 4 passed - OpenUSD net10.0: 914 passed - affected OpenUSD projects build on net10.0 and net48 Refs #4299, #4300, #4306 Fixes #4301 Fixes #4302 Fixes #4303 Fixes #4304 Fixes #4305 --------- Copilot-Session: 7cb486de-8b4c-43c5-a900-e7d2e0ad04a8 |
||
|
|
d525bc3f28 |
Bound reconnect cleanup and organize samples by domain (#4276)
# Description This PR fixes bounded ManagedSession reconnect cleanup and reorganizes the sample applications into domain-focused folders. ## Reconnect cleanup - Bounds reconnect integration cleanup with independent 30-second close and dispose limits. - Sends `CloseSession` through the awaited transport request path during `Session.DisposeAsync`. - Preserves explicit `Session.CloseAsync` behavior that waits for or cancels outstanding publish requests. - Covers the disposal request path with a regression test. ## Sample organization - `samples/OpenUsd`: `GeneratorServer`, `SiteCompositionServer` - `samples/Redundancy`: `RedundantClient`, `RedundantServer`, `RedundantPubSub` - `samples/Isa95`: `MinimalIsa95Server` - `samples/MinimalApi`: `MinimalBoilerServer`, `MinimalCalcServer`, `MinimalClient` - `samples/DI`: `PumpDeviceIntegrationServer` - `samples/Lds`: `ConsoleLdsServer` - `samples/Reference`: `ConsoleReferenceClient`, `ConsoleReferenceServer` - `samples/PubSub`: `ConsoleReferencePubSubClient` `UA.slnx` now exposes the same requested domain groups. Project references, tests, runtime sample probes, CI workflows, Dockerfiles, Compose files, helper scripts, and documentation were updated to the new paths. `samples/Robotics/MinimalRobotServer` remains under `Robotics`. ## Validation - `dotnet build UA.slnx` - 3 Redundancy short-haul sample tests - 70 DI sample tests - 914 OpenUSD tests - 1 WotCon model-materialization test - 137 AOT tests - `docker compose config` for all 6 moved Redundancy Compose files ## Related Issues - Fixes #4273 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce new build or analyzer warnings. - [x] I built the complete `UA.slnx` solution locally. - [ ] I ran all tests locally against at least .NET Framework and .NET 10. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <mschier@microsoft.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Copilot-Session: a2c2838f-74ae-4997-a1d4-50574d9e0636 |
||
|
|
397db603db |
Add OPC UA for Asset Administration Shell: the published OPC 30270 and v3 draft (#4244)
# Description Implements the **Asset Administration Shell** companion specifications on top of `marcschier/wot-type-binding`, in two halves that a Server chooses between: * **AAS V3** — the [draft](https://github.com/marcschier/opcua-drafts/tree/main/companion-specs/AAS) (`http://opcfoundation.org/UA/I4AAS/v3/`), all sixteen conformance units: the metamodel, the registry built on the abstract xRegistry base, generational materialization, federation, packages with digest-bound integrity, the Digital Product Passport vocabulary, and the Annex F bridge to WoT Thing Descriptions. * **AAS V2** — the published [OPC 30270](https://reference.opcfoundation.org/specs/OPC-30270/full) (`http://opcfoundation.org/UA/I4AAS/`), which maps the AAS V2.0.1 metamodel: object model, ingestion of XML, JSON and AASX, materialization, server and client. The two are different metamodel generations rather than two revisions of one model — V2 keeps `Asset` as a first-class Identifiable, `identification.idType`, Views and ordered collections, none of which survive into V3 — so they are **alternatives**. A Server hosts one or the other; this is documented rather than enforced at startup. Each is named for the generation it maps (`Opc.Ua.Aas.V2`, `Opc.Ua.Aas.V3`, `AddAasV2Server`, `AddAasV3Server`), and what genuinely serves both carries no marker and sits at the root of `Opc.Ua.Aas`: the deterministic NodeId encoding and idShortPath conventions, `AasOptional<T>`, the digest rule, the materialization diagnostics, the AASX container walk, and the value and operation providers, which are keyed on `NodeId` and `Variant` and so carry no version at all. ## What is in here **New libraries**: `Opc.Ua.Aas`, `Opc.Ua.Aas.Server`, `Opc.Ua.Aas.Client`, `Opc.Ua.Aas.WoT`, plus `samples/Aas` and `docs/Aas.md`. **Enabling changes to existing code**, each needed by the above: * The OPC UA `Decimal` DataType added to `Opc.Ua.Types`, with its Part 6 §5.1.10 wire encoding. `BinaryDecoder` gained one internal member, `TryReadRemainingBodyBytes`, because a trailing field with no self-describing length cannot otherwise be read; `Decimal.Decode` is its only caller. Part 6 Table 3 types the unscaled `Value` as an **OctetString whose byte count is `Length` − 2**, not a length-prefixed `ByteString`, so it carries no length of its own — `DecimalEncodingTests` pins the body byte-for-byte against that mistake, and against the `Length ≤ 2` values the same table calls invalid. * The xRegistry projection engine extracted into `Opc.Ua.XRegistry.Server` and `Opc.Ua.WotCon.Server` migrated onto it (−848/+387 lines), with the WoT Connectivity tests passing **unedited**. * Two source-generator fixes, both of which made a model impossible to build (see below). ## Defects found and fixed along the way Running the stack end to end, rather than trusting per-area test suites, surfaced defects that no unit test caught. The ones worth a maintainer's attention: * **The generator emitted non-terminating constructors.** A structure field with no declared default was initialized with `new T()`, so a self-referential DataType (`AASReferenceDataType.referredSemanticId`) recursed until the stack overflowed and `AddOpcUaAas` could not load the model at all. The test host *died* rather than failing. Only the field that closes a cycle is now left null. * **The generator omitted `new` on a redeclared placeholder.** `AASOrderedSubmodelElementCollectionType` redeclares the `SubmodelElement` slot of its base type, and the generated `Add` method hid the base one without saying so, so the V2 model would not compile. No existing model could have hit either defect — both are build failures, so anything that builds today is unaffected. * **The metamodel half had never worked outside its unit tests**: the emitted NodeSet declared no reference-type aliases and was rejected by the loader; the Mandatory `Invoke` Method was never emitted, because a NodeSet is imported as written rather than instantiated from its type; the runtime bound at the authored namespace index instead of the one the Server assigns; and `MethodDeclarationId` was addressed in the wrong namespace. Every materializer test inspected the in-memory object graph, which cannot see whether the graph is loadable — two tests now import it exactly as `RuntimeNodeSetNodeManager` does. * **The client could not see any Property member.** It browsed `HasComponent` with subtypes, but `HasProperty` is a sibling under `Aggregates`, so reading a value failed and invoking failed argument validation. * **Every repeated registry write forked a duplicate group and resource** instead of adding a version, because the identifier was re-derived against a sibling set that already contained the entity itself. This also made the persistence bounds unreachable. * `.gitignore`'s `**/packages/*` silently swallowed the whole package-integrity implementation and its tests, which would have vanished on a fresh clone. ## Specification findings Nine findings against the V3 draft are fixed in [marcschier/opcua-drafts#75](https://github.com/marcschier/opcua-drafts/pull/75). The substantive one: clause 6.1.5's absent-versus-empty rule was **unsatisfiable** for five container fields that materialize as parent components with no node of their own, which made `AAS-LosslessRoundTrip` unachievable. OPC 30270 contradicts itself twice. Both are recorded in `AasV2ConformanceUnits.cs` and the documentation rather than smoothed over: Table 83 spells the unit `I4AAS MultiLangaugeProperty` where Table 85 spells it correctly, and Table 85 lists an `I4AAS Security` unit that Table 83 never defines. Table 84, which would assign profile URIs, is empty, so `ServerProfileArray` gains no I4AAS entry. ## Known limitation Calling an `Operation` **with arguments** answers `BadTooManyArguments`. Method resolution works from both the instance NodeId and the `MethodDeclarationId`, and the argument definitions are published, but `NodeState.AddChild` does not route through `MethodState.FindChild`, so a Method imported from a NodeSet leaves its typed `InputArguments` null. Closing this means changing how the shared NodeSet import claims typed children, which affects every consumer of runtime NodeSets and is out of scope here. It is documented in `docs/Aas.md` §11, the conformance matrix marks `AAS-OperationInvoke` partial rather than claiming it, and `samples/Aas` exercises the path and reports the status rather than working around it. V2 is unaffected — its `Operation` is an OptionalPlaceholder with no declared arguments. ## Related Issues - No tracking issue yet. Opening as a **draft** for design discussion first, per the ADR guidance below; happy to open one and link it once the shape is agreed. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. ### Verification | Check | Result | |---|---| | AAS tests | **670 passed, 0 failed** on net472, net48, net8.0, net9.0, net10.0 | | Coverage | Client 80.8 %, Server 81.2 %, `Opc.Ua.Aas` 82.0 %, WoT 82.4 % — all above the 80 % gate | | Libraries | `Opc.Ua.Aas`, `.Server`, `.Client`, `.WoT` build clean on every target framework, zero warnings | | Regression | `Opc.Ua.Types` 8677, `Opc.Ua.WotCon` 1094, `Opc.Ua.XRegistry` 198, `Opc.Ua.Server` 4050 — all green | | Sample | `samples/Aas` builds warning-free and runs end to end | | Merge validation | Solution builds clean on net10.0 and net48; WoT Types 504, WotCon 1111, AAS 670, XRegistry 198 tests passed | | CI | All 99 checks green on `ff9298e44`; only the CLA signature status is outstanding | ### Review pass A full review against both specifications, and for API consistency and correctness, produced twelve fixes. Each one has a test that fails without it. | Defect | Where | Why it mattered | |---|---|---| | V2 projection aborted on **every** `File` read from a document | `AasV2EnvironmentRuntime` | The runtime bound `File/Value` unconditionally, the materializer emitted it only with content, and the builder throws `BadNotFound` for an unknown NodeId — aborting the whole registration. No reader ever populates `AasFile.File`, so no AAS V2 document could be hosted at all. | | `aas-suppl` anchored on the wrong part | `AasxPackageReaderCore`, `AasxPackageWriter` | IDTA and the Package Explorer anchor it on the environment part; we used the origin part. Every real `.aasx` returned zero supplementary files, silently. Affects both halves. | | Federation failed closed on pooled connections | `AasFederation` | `ConnectCallback` fires only for new connections, so the second fetch to an origin — and all of net472/net48/netstandard2.1 — failed with a security-flavoured error. The peer is now validated *inside* the callback, which makes reuse safe by construction. | | Malformed `Decimal` escaped as `ArgumentOutOfRangeException` | `BinaryDecoder` | A declared body shorter than `Scale` produced a negative read length. Two of the three test cases had been passing for the wrong reason. | | V2 and V3 cross-wired their value providers | `OpcUaAasServerBuilderExtensions` | Both registered `IAasValueProvider`; whichever was added first served both AddressSpaces. V2 now has its own contract, so co-hosting is harmless rather than silently wrong. `EnvironmentFolder` was also bound and ignored on the V2 path. | | Export filter skipped three nesting kinds | `AasEnvironmentExporter` | Content under `Entity`, `AnnotatedRelationshipElement` and `Operation` was exported unevaluated *and* stamped with a Digest presenting it as complete (clause 6.5.10). List authorization paths also shifted after a denial. | | Concealment held at the Method but not in the projection | `AasRegistryModel` | `GetSubmodel` answered `BadNotFound` while an anonymous Browse revealed the node, its identifier, semanticId and digest (clause 6.5.7). | | File handles were unsynchronized and unowned | `AasElementFileManager` | The async node manager holds no lock, so concurrent Calls raced a bare `Dictionary`; sequential handles let any Session read, seek or commit another's. Writes were also unbounded. | | Projection host was unimplementable outside the stack | `AasEnvironmentProjection` | The handle demanded a type with an internal constructor — our own tests used `FormatterServices.GetUninitializedObject` to get round it. Now mirrors WotCon's interface-typed registration. | | Embedded `FileType` omitted 7 of 10 Mandatory members | `AasEnvironmentMaterializer` (V2) | OPC 10000-3 6.4.4 requires them all; without `Size` and `OpenCount` a Client cannot size a read or detect an abandoned handle. | | Round-trip oracle ignored most of the model | `AasRoundTripComparer` | `Blob`, `File`, `ReferenceElement` and `BasicEventElement` had no case arm, so their fields compared equal by falling off the switch; `valueType`, `semanticId`, `entityType` and `Key.type` were never read. The clause 6.4 losslessness claim rested on it. | Known, unchanged: calling an `Operation` with arguments answers `BadTooManyArguments`, because `NodeState.AddChild` does not route through `MethodState.FindChild`. Documented in `docs/Aas.md` §11. Not addressed in this pass, and worth a maintainer decision: OPC 30270 §9.2 says all NodeIds of Nodes not defined in the companion specification **shall not** use the standard namespaces, and Table 87 marks a vendor-specific instance namespace Mandatory. Both halves currently mint instances in the companion namespace. Separating them touches NodeId minting, the runtime and the client together, so it is better done deliberately than folded into a review pass. This is a large change and touches shared code (`Opc.Ua.Types`, the source generator, the xRegistry projection). It is deliberately a draft: the enabling changes are separable if maintainers would prefer them split into their own PRs. The latest `master` is merged into this branch in `0dd0ff0e1`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8 Copilot-Session: c3d88950-4a65-4ff6-87d2-1f545a875c9b Copilot-Session: b8ab3763-b9bd-46ae-b0ef-045db26e5782 |
||
|
|
333de8305b |
Add kitchen-sink coverage NodeSet2 with three load pipelines (#4251) (#4272)
# Description Regression fence for the NodeSet2 import + code-generation surface. This adds a single, purpose-authored "kitchen-sink" NodeSet2 (`Opc.Ua.CoverageTest.NodeSet2.xml`) plus a small dependent secondary model that deliberately flip a broad set of the knobs defined by `UANodeSet.xsd` (every node class, every `Definition` shape, custom reference types, all five modelling rules, scalar/array/matrix values, a deep branching instance tree, and a wide set of reference relations). The same bytes are then loaded into **real** servers (`ServerFixture<T>` + `ReferenceServer`) through three independent pipelines and validated node-by-node, attribute-by-attribute, reference-by-reference: 1. **Source-generation, hand-written manager** - the XML is consumed at compile time via `<AdditionalFiles ModelSourceGeneratorPrefix="Opc.Ua.CoverageTest">`; the generated `AddOpcUaCoverageTest(context)` composes the predefined nodes inside a `CustomNodeManager2`, so every node materialises as its generated `NodeState` subclass. 2. **Runtime import** - the identical XML is imported at startup through `RuntimeNodeSetNodeManagerFactory` / `AddRuntimeNodeSet`. 3. **Source-generation, generated fluent manager** - a `[NodeManager]`-attributed partial makes the generator emit the whole `FluentNodeManagerBase` manager, factory and typed fluent-builder surface. A shared, data-driven `[TestCaseSource]` battery runs the identical assertions against all three servers, and a cross-pipeline equivalence fixture asserts the three address spaces agree. A dependent **second namespace** (`.../Secondary/`) exercises cross-namespace `HasTypeDefinition`, `HasSubtype` and `HasComponent` references in every pipeline. Authoring the model surfaced two latent source-generator bugs, both fixed here: - `NodeSetToModelDesign` crashed on a top-level `<ServerUris>` table (`StringTable.Append(null)` for the reserved local-server slot). Added a null guard plus a focused regression test (`NodesetServerUrisTests`). - `FluentBuilderGenerator` emitted colliding typed-builder accessors for any model with more than one structure encoding, because `DataTypeEncoding` objects hang off their `DataType` and surfaced as parent-less "root" instances sharing the reserved `DefaultBinary`/`DefaultXml`/`DefaultJson` symbolic names. They are encoding metadata, never fluent-wired, so they are now excluded from the typed builder. A few edges are intentionally not asserted for cross-pipeline equivalence because the pipelines legitimately differ (documented in `CoverageNodeSet/README.md`): type-dictionary / `DataTypeDescription` nodes (the generator regenerates schema rather than emitting them), a handful of ambiguous duplicate-hierarchical / View-`Organizes` edges, and runtime structure stand-ins (structure definitions are validated via the `DataTypeDefinition` round-trip instead). ## Related Issues - Fixes #4251 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> |