UA-.NETStandard

mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-28 22:55:49 +08:00

Author	SHA1	Message	Date
Marc SchierandCopilot	fb80bfdc82	[WotCon] Track WoT Connectivity/Binding 1.1 and add projections (Section 12) (#4182 ) Follow-up to #4128, which merged the WoT Connectivity 1.1 work against draft2 of the companion specifications. Both drafts have moved since, and this branch tracks them through to draft3, adds the projection feature they introduced, and closes the gaps a bidirectional re-audit found. The specifications are the private [`OPCF-Members/spec-drafts`](https://github.com/OPCF-Members/spec-drafts) drafts of OPC UA — WoT Connectivity 1.1 and OPC UA — WoT Binding 1.1. They are still drafts, which is why this is opened as a draft PR — see [Status](#status) at the bottom. ## What changed in the specifications Two revisions landed upstream while #4128 was in flight. draft2 replaced the `eventGroup` / `actionGroup` grouping constructs with a single projection / view concept, added fifteen Section 6 vocabulary terms, and made two identifier rules normative. draft3 ([spec PR #8](https://github.com/OPCF-Members/spec-drafts/pull/8)) un-deprecated the incorporated OPC 10100-1 v1.02 surface, restructured the conformance clause into three profiles, and tightened two security obligations. The draft3 NodeSet is structurally identical to draft2 — I diffed all 286 nodes node-by-node. Every change is metadata: 96 `ReleaseStatus="Deprecated"` attributes removed, `PublicationDate` advanced, and `WOTC-ProjectionMaterialization` claimed by three Type Nodes that previously implemented it in name only. ## Projections (WoT Binding Section 12) The substantial addition. A projection document selects affordances from other Thing Descriptions or Thing Models and resolves to a document of its own; materialized, it becomes an OPC UA `View` that creates no affordance Node and only `Organizes` what its sources already materialized. \| Piece \| Where \| \|---\|---\| \| Document model and validation \| `Opc.Ua.Types/Wot/WotProjection.cs`, `WotProjectionModel.cs` \| \| Four-stage resolution engine \| `Opc.Ua.Types/Wot/WotProjectionResolver.cs` \| \| Materialization plan \| `Opc.Ua.WotCon.Server/Materialization/WotViewProjectionPlan.cs`, `WotProjectionViewBuilder.cs` \| \| Live address-space host \| `WotProjectionViewNodeManager.cs`, `LifecycleWotViewProjectionHost.cs`, `IWotViewProjectionHost.cs` \| \| Node lookup seam \| `IWotMaterializedNodeIndex.cs` \| Groups reached by `ua:Organizes` become organizational Objects rather than nested Views, only the outermost materialization is a `View`, and a member whose source is served by another server is omitted and reported rather than recreated — the federation case the clause calls out. ### How this is validated The specification publishes twenty worked examples, two of which are a golden pair: a projection document and the resolved view it is defined to produce. `WotSpecExampleTests` embeds all twenty and runs the pair through the resolver, asserting against the specification's own expected output rather than against my reading of the prose. That single document exercises all three selection forms, the bulk naming rule, the security closure naming and the provenance term. This is the strongest evidence in the PR, and it is worth more than the unit tests around it: it is the same check the upstream `validate_local.py` performs. ## Other spec work - Section 6 vocabulary — fifteen missing model and platform terms with their validation rules (`WotNodeSetConverter.ModelVocabulary.cs`). - Portable identifiers — the two rules release 1.1 made normative, with `WotNodeSetConverterOptions.AllowNonPortableIdentifiers` as a documented downgrade-to-warning switch for documents authored against v1.00. Defaults to `false`. - xRegistry §6.9 identifier construction (`XRegistryIdentifier.cs`) — reverse-authority symbolic identifiers; all seven published worked examples are test cases. ## Gaps found by re-auditing against draft3 I audited in both directions — every normative requirement to code, and every `uav:` term in code back to a spec that defines it. Three findings, all fixed: 1. `Wot-Con 1.02` accepted anything well-formed. draft3 makes format validation part of the unit, because WoT-Con Minimal is one unit wide and materialization projects a valid document. The 1.02 upload path only deserialized into the `ThingDescription` POCO, which happily accepts JSON that is not a WoT document at all — every member of that type is optional, so `{}` deserialized and was materialized. It now parses via `WotDocument.Parse` and requires the document to identify itself by a non-empty `name` or `title`, returning `Bad_DecodingError` and materializing nothing otherwise. The rule lives in `ThingDescriptionFormatValidator` and is shared with `CreateAssetForEndpoint`, whose auto-generated Thing Descriptions §11 also treats as untrusted input. 2. Two `WoTFile` operations were ungated. draft3 §11 names `Write` and `CloseAndUpdate` alongside the five management methods; both reached the materializer without an access check. `EnforceManagementAccess` is now wired into `WotAssetFileManager`. 3. Two invented `uav:` terms. `uav:NodeSet2Preservation` was declared and never used — removed. `uav:eventFields` is read by `OpcUaBindingPlanner` and defined in neither spec, while a comment claimed it came from OPC 10101. It still works for compatibility but is now documented as a non-standard extension at both sites rather than misattributed. The SSRF prohibition is satisfied by construction: there is no HTTP client anywhere in these assemblies, and all resolution goes through the injected `IWotThingResolver`. ## Two ambiguities fixed in the specifications themselves The audit turned up two places where a conforming implementer had to invent an answer. Rather than pick silently, I took both upstream — they are pushed to [spec-drafts PR #8](https://github.com/OPCF-Members/spec-drafts/pull/8) (all checks green) and are now normative in Binding §12.6 and Connectivity §7.13: - `ViewVersion` had one sentence saying only that it changes. It now changes iff the resolved membership changes, is updated at commit time alongside the `NodeVersion` stamping, and is a deterministic function of the membership alone in a canonical order — so two servers that resolved the same membership agree, which a per-server counter would not across a redundant pair. Explicitly not monotonic: compare for inequality only. - "Type-level view" was used once in §12.2 and defined nowhere, while every other statement of materialization was universal. §12.6 now defines it rather than contradicting §12.2: the NodeClass is `View` in both cases. Both were already what this code did, so neither changed behaviour — but the order-insensitivity the clause added was not covered by any test, and `ViewVersionIsUnchangedWhenOnlyTheOrderOfTheMembershipChanges` now holds it. It is mutation-verified: removing the ordinal sort turns it red. `docs/WotBindings.md` no longer lists these as points "the specification leaves open". ## Validation \| Suite \| Result \| \|---\|---\| \| `Opc.Ua.Types.Tests` \| 8525 ✅ (8518 on net48) \| \| `Opc.Ua.WotCon.Tests` \| 1057 ✅ (net10.0 and net48) \| \| `Opc.Ua.WotCon.Bindings.Tests` \| 559 ✅ \| \| `Opc.Ua.XRegistry.Tests` \| 196 ✅ \| \| `Opc.Ua.Server.Tests` \| 4027 ✅ \| \| `Opc.Ua.SourceGeneration.Core.Tests` \| 3771 ✅ \| \| `Opc.Ua.WotCon.Samples.Tests` \| 9 ✅ \| The full `UA.slnx` builds with 0 errors and 0 warnings attributable to this PR on all 6 TFMs (`TreatWarningsAsErrors` is on). `Opc.Ua.Server.Tests` and the source-generation suites are listed because this PR now also touches `MasterNodeManager` and the WoT-Con NodeSet. ## Compatibility No public API is removed or broken. Everything added is new surface using `ArrayOf<T>` / `ByteString` per the repo conventions, wired for DI with a direct-construction fallback, and the one behavioural tightening (`Wot-Con 1.02` format validation) rejects input that was never valid to begin with. One pre-existing issue is not addressed here because it came in with #4128 and is out of scope: `WotAssetFileManager.OnCloseAndUpdate` uses `.GetAwaiter().GetResult()`, which the repo rules ban. Worth a follow-up. ## Landed since this PR was opened - DI upgraded to 1.05.0. `ConnectsTo` was declared a subtype of `HierarchicalReferences`, contradicting OPC 10000-100 §5.5 Table 48. That is an upstream error in every official DI NodeSet up to 1.04, corrected by the OPC Foundation in 1.05.0, so the fixture is replaced with the verbatim 1.05.0 file rather than hand-patched. A guard test pins the corrected form. This also closed a second gap: six DI types the WoT model was missing are 1.05 additions. - The sample Pump has the type definition OPC 40223 declares for it. `Pump1.Identification` carried DI's `FunctionalGroupType` instead of `PumpIdentificationType`. Every test, the sample documents and the README agreed with each other about it, which is why it survived. - `WotPumpAddressSpaceComparisonTests` compares this server against `PumpDeviceIntegrationServer`, which builds the same OPC 40223 Pump from generated NodeSets and hand-written wiring. An independent oracle rather than a restatement of the same assumptions - it is what found the defect above. - Error containment in `MaterializeProjectionViewsAsync`. An exception mid-loop discarded the applied View handles before they were published, leaving View Nodes in the address space that nothing could remove. - `HasWoTProjection` is now created; it was documented and plumbed but never wired. - The default in-memory View host no longer deletes the View it has just applied. - An undeclared NodeSet alias now says so. It previously surfaced as `IdentifierMissing`, naming neither the value nor the fact that an alias was expected. - `ViewVersion` follows the algorithm the specification now states. My earlier spec text was rejected on review, correctly: it named neither the function nor the canonical order it demanded, ignored that OPC 10000-3 §5.4 requires a value greater than zero, and its "when, and only when" is unsatisfiable for a 32-bit value. §12.6 now specifies the algorithm outright and this implementation follows it exactly, pinned by an independently computed test vector. Implementing it surfaced a further defect in the clause - joining members on U+000A is not injective, because a NodeId string identifier may itself contain U+000A - which is fixed in the specification by length-prefixing each member. - An auto-generated Thing Description is treated as untrusted input. §11 requires it; `CreateAssetForEndpoint` validated the endpoint but handed whatever the discovery provider returned straight to the materializer. The `Wot-Con 1.02` format rule now gates both paths from one place. - Document bytes moved out of the registry snapshot into a content-addressed store, so a commit no longer rewrites the whole corpus to edit one document. Writes stage and are promoted during the commit, which preserves both durability-first ordering and the fail-closed rule that a blob directory without a manifest means operator recovery. Restoring streaming digest verification along the way closed a real gap: content integrity had degenerated to a length comparison. - External references now reach a NodeManager registered later. A reference to a Node that does not exist yet is dropped rather than queued, and the dynamic-registration path had no second phase to apply them in, so registering a NodeSet before the one it points into lost the forward edge permanently. Review of that change then found a state leak in its rollback path, fixed with a red-to-green regression test. - `WOTC-Legacy` is renamed `Wot-Con 1.02` in the specification and here. Editorial: the NodeSet diff is `<Category>` values only. ## Status Open for review. Two things may still move: 1. The specifications are unpublished drafts. Anything here can change if the working group decides differently. The `ViewVersion` definition has since been settled - §12.6 specifies the algorithm and this follows it - but the same could happen to any other clause tracked here. 2. `uav:eventFields` is kept as a documented non-standard extension. That is a judgement call the WG may want to reverse, either by adopting the term or removing it. Two known items are recorded rather than fixed: - `Pump1` has no hierarchical parent, so it is reachable by NodeId but not by browsing down from `Objects`. The cross-NodeSet ordering defect this was originally attributed to is fixed above, and cross-manager references are now symmetric in both registration orders. The Pump itself is still blocked on a different cause found while verifying that fix: with the reference added, `DeviceSet` exists but the Pump has zero inverse references, so the edge is dropped at import - before any external-reference machinery runs. That is a separate defect in the materialization path, not the one that was fixed, and it is left recorded rather than guessed at. - `WotAssetFileManager.OnCloseAndUpdate` uses `.GetAwaiter().GetResult()`, which the repo rules ban. Pre-existing from #4128 and out of scope here. Review of the projection resolver and the materialization host is the most valuable thing to look at; the vocabulary and identifier commits are mechanical by comparison. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8	2026-08-08 09:08:23 +02:00
Marc SchierandCopilot	ca89ac85a7	Add a Generators companion-spec sample and compose servers at a site level (#4180 ) # Description Adds a Generators companion-specification sample alongside the existing pump sample, and a `SiteCompositionServer` that composes several device servers into one OpenUSD scene at a supervisory level — demonstrating that servers can be aggregated without mirroring their address spaces. Three things, in dependency order: ### 1. `GeneratorServer` A self-contained server realising the draft Generators spec (`http://opcfoundation.org/UA/Generators/`) end to end: N simulated generating sets, DI + Machinery integration, a twelve-state operating state machine, four protection alarms per set, six control methods, and a per-set OpenUSD twin. The organising idea is that load fraction is the only independent variable. Everything else is a function of it: ``` V̇_f(x) = 3.67 + 100·x fuel rate [L/h] η(x) = P(x) / (V̇_f(x) · ρ · LHV) efficiency S = P / PF I = S / (√3·V_LL) f = N·p / 120 ``` This is not stylistic. When each measurement is an independent oscillator — as the pump sample's simulation once was — a server happily publishes a duty point no real machine could occupy, and no test can catch it because there is nothing for the values to be inconsistent with. Deriving them from one variable means `P = √3·V·I·PF` and `η = P/(V̇·ρ·LHV)` hold at every tick by construction. The published `DATASHEET.md`, the engineering ranges, the trip points and the simulation all read the same constants in `GeneratorDatasheet.cs`, so the document and the server cannot describe different machines. The machine is a fictitious SimGen Systems GenX-500 (400 kW prime / 440 kW standby, 400/230 V, 50 Hz, four-pole). Figures are representative of a real industrial genset class; no vendor is named anywhere in the code or docs. ### 2. Cross-server composition in the connector `RemoteSessionFactory` already existed in the client library but the connector CLI never set it, so cross-server composition was unreachable. `--federate` wires it up: the connector opens a session to each server named by a component binding, discovers its representations and drives its bindings into the same stage. It is opt-in because the endpoint the connector dials comes from the server being rendered rather than from the operator, which makes honouring it a trust decision. Federation is best-effort per component — a subordinate that is down is logged and skipped, and the rest of the scene still renders — which is the only `src/` change in this PR. ### 3. `SiteCompositionServer` Owns no devices. It publishes a site stage plus one cross-server component binding per subordinate, carrying that server's `ComponentServerUri` and `ComponentEndpointUrl`. Nothing is mirrored, so there is no cache to invalidate and no second copy of the truth. ## Notes for reviewers Everything interesting in this PR was found by running the servers, not by reading the code. Six defects, none of which looked wrong in review: - `IsShutdown` and `SubsystemName` are optional on `GeneratorProtectionAlarmType`, so the generated factory does not materialise them. `CreateOrReplace` alone produces a child that exists, appears in `GetChildren` and holds the right value — but carries no `ReferenceTypeId`, so no browse can reach it. Every alarm was publishing its trip without saying whether the trip stops the machine or which subsystem to go to. `AddXxx(context)` first is what gives the child its `HasProperty` reference. - The protections could never fire. Every trip point sits outside the band the datasheet curves produce — that is what a datasheet means — so all four alarms, the shutdown class, `ResetFaults` and the whole `Fault` branch of the state machine were unreachable while the README documented them as observable. Overload was worse: the load clamp ceiling was set to exactly the trip point, making a strictly-greater-than comparison unsatisfiable. Faults are now injectable, and the first set — the one the hero camera frames — develops one on a slow rotation. - A shutdown trip removes the condition that caused it, because oil pressure and coolant temperature are only supervised while the engine turns. Each alarm went active for one tick and cleared, leaving an operator with a stopped machine and no indication of why. Shutdown-class alarms now latch until the set leaves the shutdown state. Every unit test that checked the condition passed throughout — the defect only appears when the trip and the supervision interact over time. - `Synchronizing` and `Paralleled` were declared, drawn in the README and never entered. The first set now energises a dead bus and closes onto it while every other set synchronises to a live one — both how a real plant parallels, and what makes the two states observable. - The tick raced client method calls. The simulation tick runs on a thread-pool thread while method calls arrive on request threads, and both transition sets and write the same nodes. A tick moving a set to `Cooldown` and a concurrent `EmergencyStop` could interleave the paired `CurrentState` / `CurrentState.Id` writes and leave a client with a state name from one transition and a state node from the other — the exact failure the paired write exists to prevent, reintroduced by the threading model. Both paths now take one gate. - Low oil pressure supervised from raw speed tripped every set during cranking, because pressure has not built yet. Now gated on `IsSpinning`, which is what a real start-up bypass does. Smaller fixes: shutdown trips are applied after the whole evaluation pass (stopping mid-loop made the remaining conditions read healthy, collapsing simultaneous trips to whichever came first in the table); `ResetFaults` reports its clears as events (a client learns of condition state changes only through events, so a silent clear leaves an alarm-list client showing it forever); the start counter moved into the transition so commanded starts are counted; the federated connector closes the remote session if the connector constructor throws between taking ownership and registering it; and `prepare_machinery_nodeset.py` now asserts the IA namespace is last before removing it rather than only claiming so in its docstring — removing any earlier entry would silently renumber every namespace after it and rebind their NodeIds. A model limitation is left visible rather than papered over. `GeneratorStateMachineType` declares an emergency stop only out of `Running`, `Loaded` and `Paralleled`, so the sample refuses `EmergencyStop` from `Starting` and `Warmup`. A real panel stops from anywhere. That is the specification's shape, not this sample's choice, so it is pinned by a test and named in the docs. `Model/` vendors a reduced Machinery nodeset, derived mechanically by whitelist. The full official nodeset does not survive the model source generator (`MODELGEN003`) and drags in IA through a single optional `Stacklight` member a generating set does not have. Deriving it by script keeps the provenance checkable; the whitelist is the only thing to edit when more types are needed. Never hand-edit the generated output. The federated scene renders end to end. Getting there surfaced three more defects, all of which produced a plausible-looking scene with nothing in it: the generator sample's plant aggregation was created under an already-registered `DeviceSet` and so was invisible to every client (it had never rendered geometry, standalone or federated); the connector's asset fetch ran on the primary session only, so composition referenced layers that were never downloaded; and both generator colour bindings wrote `primvars:displayColor` to prims that did not declare it, so the renderer rejected every update while the file still looked correct. Cross-server components compose under the subordinate's own root, so the site layer now places `/Plant` and `/Powerhouse` rather than leaving them stacked on the origin. `OpenUsdRepresentation` is now mounted with `HasAddIn`, not `HasComponent` (review feedback). The nodeset says so explicitly — "Mounted with HasAddIn" — but every server in the tree used plain `HasComponent`. It survived because `HasAddIn` is a subtype of `HasComponent`: the representation still browses, still aggregates and still drives a twin, so every functional test passed before the fix and after it, and only a conformance checker could tell. Seven mount sites corrected. `PumpDeviceIntegrationServer` and `MinimalRobotServer` had hand-rolled the mount instead of calling the shared `CreateRepresentation` helper, which is exactly how they drifted from it. Both sample E2E suites now assert the reference type across every discovered representation — a narrower check would have missed the plant-level representation that arrived later from master still carrying `HasComponent`. Live colour is not achievable in the current OpenUSD viewer, and that is now documented rather than worked around. Proven with a standalone probe: `primvars:displayColor` resolves as `color3f[]` from the `UsdGeomGprim` schema whatever the layer declares, and the `OpenUsd 0.4.0-alpha` managed API has no writer for that type. Bound `UsdPreviewSurface` materials are not shaded either, so geometry renders grey unless it also carries an explicit `displayColor` primvar. The visibility bindings carry no such caveat and are what the samples rely on to show state. Filed upstream as [openusd-dotnet#2](https://github.com/marcschier/openusd-dotnet/issues/2), [#3](https://github.com/marcschier/openusd-dotnet/issues/3) and [#4](https://github.com/marcschier/openusd-dotnet/issues/4), and linked from `docs/OpenUsd.md` so a reader can check whether they have since been fixed. ## Related Issues No tracking issue — this is additive sample and documentation work with a single small, self-contained change to `src/` (per-component error isolation and session-ownership hardening in `OpenUsdConnector.Composition.cs`). Happy to open one if maintainers would prefer it tracked. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. ### Testing The generator fixtures live in `tests/Opc.Ua.OpenUsd.Tests/Generator` — 82 tests, no new project. `Opc.Ua.Di.Tests` would have been the natural home next to the pump fixtures, but both device samples source-generate their own reduced `Opc.Ua.Machinery` model and expose it via `InternalsVisibleTo`, so referencing both from one assembly makes every Machinery type ambiguous (CS0433); `Opc.Ua.OpenUsd.Tests` references no other sample and already hosts `RobotAssetContractTests`. The suite runs 870 on net10.0 and net48 (784 existing + 86) and 788 on net472, where the sample does not exist and only the framework-independent asset-contract tests run — forgetting exactly that exclusion broke CI on the sibling pump PR. The tests target the things in this sample that fail quietly: - Datasheet conformance holds the model to its own claim — rated speed is `120·f/p`, rated current follows from the rating, the fuel curve reproduces the published table, and `η = P/(V̇·ρ·LHV)` reconciles at every load. Efficiency is swept across the whole simulated range to show it stays inside (0,100) and rises monotonically; a fit that goes negative somewhere in its range is a coincidence that happens to look right at the duty point, not a model. A further test reads `DATASHEET.md` and checks the document quotes the figures the server actually serves. - Drift between the two descriptions of the state machine — the physics' `IsLegalTransition` and the model's `GeneratorStateMap` — is checked in both directions. A transition the physics permits but the map lacks moves a machine without telling a client; one the map holds but the physics refuses is dead weight that looks supported. - Reachability at run time, not just in the declared table — the test that catches states the model declares and nothing ever enters. - That every protection can actually fire, which is the half of the alarm contract that "a healthy set annunciates nothing" does not cover. A hosted end-to-end fixture connects a real client and asserts what the address space actually exposes — the aggregation at `/Powerhouse/Generators`, its component binding, and every per-set twin. That is the only kind of test that catches the registration defect above: the node object had the right browse name, binding and asset reference, so every test that inspected objects passed while no client could see it. Every state-machine and protection fix was checked by mutation. Restoring the clamp ceiling, the direct `Running → Loaded` path, the old start-counter placement, or removing the alarm opt-in each makes the corresponding test fail, naming the specific defect. A test that has never been seen to fail has not been shown to work. Verified against a running plant: `Synchronizing`, `Paralleled` and `Fault` all observed on a two-set server, with `HighCoolantTemperatureAlarm` seen active on the fault subject. Method behaviour verified separately — per-set distinct `CurrentState` nodes, `EmergencyStop` → `EmergencyStopped` with `Start` then refused as `BadInvalidState`, `ResetFaults` → `Off`, `SetOperatingMode(9999)` refused with the mode unchanged, and the other sets untouched throughout. Also verified as a three-server federated stage. 0 warnings on a clean Release rebuild of both samples, the client library, the connector and the tests, across every TFM. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe608993-f3c9-4779-a6b9-9a9eabfc24ba	2026-08-07 10:48:02 +02:00
Marc SchierandCopilot	416d619c41	Split the CI matrix across both systems, split the required test gate from the advisory coverage check, and give background work an owner (#4158 ) # Description Two changes to how CI is routed and how it reports. ## 1. Split the CI matrix back across both systems Flips both halves of the CI backend switch from `ado` to `actions`, so the workload is shared instead of concentrated on the managed pool. The switch is checked into source in two places that must always be flipped together: \| File \| Setting \| Before \| After \| \| --- \| --- \| --- \| --- \| \| `azure-pipelines.yml` \| `parameters.ciBuildBackend` \| `ado` \| `actions` \| \| `.github/workflows/buildandtest.yml` \| `env.CI_BUILD_BACKEND` \| `ado` \| `actions` \| \| Work \| GitHub Actions \| Azure Pipelines (managed pool) \| \| --- \| --- \| --- \| \| All-TFM solution builds \| `build-all-tfm-windows` + `-linux` \| two-TFM PR gate (`net10.0`, `net48`) \| \| Test matrix \| ubuntu, every test project \| Windows `net48` + `net10.0` fast PR legs \| \| Native AoT \| ubuntu \| Schedule/Manual only \| \| macOS \| always (no managed-pool macOS image) \| — \| ## 2. Split the required gate from the advisory coverage check The coverage gate job was appended to every test stage, so `Fast PR test` emitted three sibling checks whose display names collided — `Coverage Release (net10.0)` appeared twice, once for the Windows leg and once for macOS, with nothing to tell them apart: ```text ✓ Coverage Release (net48) 55s ● Coverage Release (net10.0) ○ Coverage Release (net10.0) ``` That job also fused two unrelated concerns: whether the tests passed (which must block a merge) and whether coverage met its thresholds (which should not — the `net10.0` leg had `enforceCoverage: true`, so a coverage dip blocked merges). Both concerns now get a stage of their own, mirrored in both CI systems: \| Concern \| Azure Pipelines \| GitHub Actions \| In the branch ruleset? \| \| --- \| --- \| --- \| --- \| \| Every test passed \| `Tests passed` stage \| `build-and-test summary` job \| Yes — required \| \| Coverage meets thresholds \| `Code coverage` stage \| `code coverage` job \| No — advisory \| The coverage check reports a clean failure when the thresholds are missed, so the miss is visible, but it never blocks the merge. Coverage is now merged once across every leg of the run instead of once per leg, which also removes the `enforceCoverage` / `publishCoverage` "exactly one invocation may set this" parameters. Neither check re-runs any tests — the previous design re-ran the whole suite in one job with the collector attached, which serialised a suite that is deliberately fanned out and always exceeded the stage timeout. ### Coverage checks now exist in GitHub Actions `build-and-test` already ran with `--collect:"XPlat Code Coverage"` and uploaded the fragments, but nothing ever merged or evaluated them. It now runs the same `check-coverage.ps1` against the same `coverage-thresholds.json` as Azure. Since `CI_BUILD_BACKEND` defaults to `actions`, that ubuntu matrix — which runs every test project — is the fullest coverage signal in the repository. ### Reporting the numbers `check-coverage.ps1` gained `-SummaryPath`, rendering a markdown report that both systems surface: - GitHub Actions — appended to the job summary and upserted as a single sticky pull-request comment, updated in place on each run. Threshold misses also appear as `::error::` annotations. - Azure Pipelines — attached to the build summary via `##vso[task.uploadsummary]`. The report is a pass/fail table of the project floor, patch coverage and baseline delta, with the uncovered changed lines listed by file in a collapsible block. Rendered example: > ## Code coverage > > ❌ Coverage gate failed. This check is advisory and does not block the merge. > > \| Check \| Result \| Threshold \| > \| --- \| --- \| --- \| > \| ✅ Project line rate \| 72.10% (40 731/56 496 lines) \| >= 70.00% \| > \| ❌ Patch coverage \| 0.00% (0/1 changed lines) \| >= 75.00% \| The comment is fork-safe: a `pull_request` from a fork gets a read-only token regardless of the `permissions:` block, so it degrades to the job summary alone. The sticky comment uses `actions/github-script` rather than a community action, keeping this workflow on first-party actions only. ## Notable correctness details These were found and fixed during review, and are the parts most worth a second pair of eyes: - *`dependencies.<stage>.result` resolves at stage* scope only. At job scope `dependencies` refers to other jobs of the same stage, so reading stage results from a jobs template would have silently returned empty and the gate would have passed everything. `gate.yml` and `coverage.yml` are therefore stage templates that derive their own `dependsOn` from the same stage list they evaluate, so the two can never drift apart. An unresolved result is a hard error. - Both rollups run on `not(canceled())` / `always()`, not on success. A check that reports skipped is treated by GitHub as satisfied, so a required check that skips when its dependency fails would wave a red build straight through. - `get-matrix.ps1` used to emit an empty matrix on a discovery miss.** The downstream job is then skipped, a skipped job rolls up as a succeeded stage, and the gate would have approved a run that executed no tests. It now fails, with an `-AllowEmpty` opt-out. - Zero coverage reports used to be a green advisory check, which would hide a broken collector indefinitely. Both systems now distinguish "the tests never ran" (warning) from "the tests ran and published nothing" (error). - The gate reads stage-level results rather than enumerating matrix-generated job names, which change whenever a test project or agent is added. ## Test-host hang and the reconnect defect behind it — both fixed Flipping `CI_BUILD_BACKEND` to `actions` started exercising the ubuntu matrix, which `master` never runs, and it immediately surfaced two real defects. 1. The test host hung for ten minutes. `Assert.ThrowsAsync` blocks the calling thread with no timeout, and these tests drive a fake clock from that same thread — so once the reconnect parked on a fake-clock timer, nothing could ever advance it. NUnit sat in `WaitForCompletion` until `--blame-hang-timeout` killed the host, aborting the run at 3902 of 4238 tests and taking every other test in the assembly down with it. A hang dump named the culprit exactly. Replaced with an awaited, bounded `AssertThrowsAsync` helper (the repo's own rules forbid sync over async). The run now completes in ~1m29s with all tests executed. 2. A budget-aware reconnect never returned. With the hang gone, two tests failed with `Expected ServiceResultException but got TimeoutException`. `dumpasync` on a hang dump gave the whole chain: ``` ReconnectAsyncWithBudgetShrinksDelayToFitRemainingAsync → AssertThrowsAsync → ClientChannelManager.ReconnectAsync → ClientChannelManager.ReconnectLeaseAsync → ClientChannelManager.SwapFaultedEntryAsync ← parked here ``` `master` (via #4128) added `IsTerminalReconnectRace` so a reconnect that loses a race against a concurrent close recovers on a freshly swapped entry. The predicate only asks whether the entry ended up `Closed`/`Faulted` with `BadSecureChannelClosed` — which is also exactly how a reconnect ends when the caller's retry budget or the reconnect policy is deliberately exhausted. So a budget-exhausted reconnect was misread as a race: the manager swapped the entry and started a second, unbudgeted cycle, waiting out the swap back-off first. `ReconnectAsync(channel, budget)` therefore could not return within its budget, and under a fake clock it never returned at all. The fix requires the budget to have room left before treating a terminal failure as a race: ```csharp return !ct.IsCancellationRequested && !budget.IsExhausted && sre.StatusCode == StatusCodes.BadSecureChannelClosed && entry.State is ChannelState.Closed or ChannelState.Faulted; ``` The parameterless `ReconnectAsync(channel, ct)` overload passes an unlimited budget, so genuine race recovery is unchanged. Verified locally: Core 4152, Client 2122 and Sessions 776 tests all pass, and the full GitHub Actions PR run is green across 56 jobs. Two smaller fixes came along the way, neither weakening an assertion: - `ObservableFakeTimeProvider` waiters made relative ("N more timers from now") rather than absolute ("the Nth timer of the run"). Absolute numbering let an earlier reconnect consume the slots, so a waiter completed before the awaited timer existed and `Advance()` fired nothing. - The raw worker thread in `NodeStateHandlerConcurrencyTests` was a foreground thread whose stop flag was read without a barrier and set only on the success path — a failed assertion leaked a spinning thread that pins the process at exit. Now background, `Volatile`, stopped in a `finally`, joined with a bound. ## Background work now has an owner The reconnect hang above was one symptom of a pattern the codebase had 26 times across seven assemblies: a bare `_ = Task.Run(...)`. Each one handed work to the thread pool and forgot it — nothing observed the exception if it threw, nothing bounded how many ran at once, and disposal raced them, so a component could finish tearing itself down while work it started was still touching its fields. Two of those sites already carried a standing `// TODO: Await the task completion in shutdown`. `BackgroundTaskScope` (in `Opc.Ua.Types`) gives them an owner: schedule with `Run`, drain with `DisposeAsync`, or signal-only with `Dispose` where the owner's teardown is synchronous and awaiting would be sync over async. `Run` never blocks and never throws — which matters, because most of these call sites went to the thread pool precisely to escape a lock they were holding. 11 unit tests cover drain, cancellation, exception observation, the concurrency cap, post-shutdown rejection and idempotent disposal. Two of the fixes were more than re-wiring: - `SessionManager` / `SubscriptionManager` loops were structurally unawaitable. Both return `ValueTask`, so `Task.Factory.StartNew(() => ...)` handed back a `Task<ValueTask>` that completes at the first yield. Storing it would not have helped — awaiting it would only have awaited the scheduling. Both now go through `AsTask().Unwrap()`, matching the neighbouring `StartConditionRefreshWorker`. - `ChannelAsyncOperation` was swallowing subscriber exceptions. Its `doNotBlock` branch had no try/catch while the inline branch beside it did. It is per-operation with no owner to hang a drain on, so it gained the missing handler rather than a scope. The Kubernetes readiness endpoint keeps a purpose-built semaphore instead: an unauthenticated endpoint needs a hard cap on concurrent handlers, not just an owner. ### ⚠️ Breaking: `ISessionManager.Shutdown()` is removed Replaced by `ShutdownAsync(CancellationToken)`. There is no correct synchronous way to wait for the session monitor loop, so leaving the overload would only have preserved the race. This mirrors `ISubscriptionManager`, which already had `ShutdownAsync`. Migration notes are in `docs/MigrationGuide.md`; only one in-repo implementer existed. Also converted the injected `object partitionLock` shared by `LogicalSubscription` and `CompositeMonitoredItemCollection` to `System.Threading.Lock`, per the repo rule. ## Patch coverage now scales with the size of the patch A flat floor over changed lines punishes small changes for arithmetic: two uncovered lines in a four-line fix reads as 50 % and failed an 80/5 floor — which is exactly how this branch earned a red coverage check for a four-line predicate change. A check that fires on changes nobody considers undertested is one people learn to ignore. \| Coverable changed lines \| Floor \| Below it \| \| --- \| --- \| --- \| \| 1 – 10 \| 50 % \| warning, still passes \| \| 11 – 100 \| 60 % \| warning, still passes \| \| more than 100 \| 75 % \| failure \| Only changes past the last band can fail. Below it the author still gets a warning naming the uncovered lines. Bands live in `patch.bands` in `coverage-thresholds.json` and each carries its own `enforced` flag. Verified end-to-end against synthetic repositories at every boundary. ## Codecov is back, as reporting Dropped in #4122 when the in-pipeline gate replaced it. Keeping the gate was right, but dropping Codecov also lost the PR comment, the file-by-file diff view and the trend. It returns with both statuses `informational: true` — two gates with two sets of thresholds would eventually disagree, and `coverage-thresholds.json` stays the single source of truth. The upload is switchable and can never fail a build: `enableCodecov` (Azure) and `ENABLE_CODECOV` (Actions), both defaulting on, both skipped when `CODECOV_TOKEN` is absent as on fork PRs. Each system uploads the single merged report it already builds, under its own flag. The old `codecov.io/bash` uploader was sunset, so Azure uses the current CLI. ## Follow-up actions for maintainers 1. Update the branch ruleset. The per-leg `Coverage Release (…)` checks are gone. Azure reports checks as `<pipeline> (<stage> <job>)`, so the exact names are: \| Check \| Require? \| \| --- \| --- \| \| `OPCFoundation.UA-.NETStandard (Tests passed Verify stage results)` \| yes \| \| `build-and-test summary` \| yes (already required) \| \| `OPCFoundation.UA-.NETStandard (Code coverage Merge and evaluate)` \| no \| \| `code coverage` \| no \| 2. Re-calibrate the thresholds. `coverage-thresholds.json` is still seeded from the last Codecov reading (73.61 %). The two systems now report different numbers by design — Actions merges every project on ubuntu, Azure only the Windows fast-PR legs, and scheduled runs read higher still because the Debug / .NET 8 / .NET 9 / netstandard stages also contribute. Re-seed from the GitHub Actions figure. 3. Note: a failed advisory coverage stage still turns the whole Azure run red, which the README build badge reflects on `master`. That follows from wanting a visible red X on a threshold miss; say the word if you would rather it were a warning. 4. The Azure agent pool is the slowest thing here. Several runs on this PR sat queued for hours, and one build failed with `Insufficient system resources` while writing an XML doc file — an agent exhaustion error, not a code error. It passed on re-run. Worth raising against the `netstandard` Managed DevOps Pool capacity separately. ## Related Issues _No tracking issue; this is CI infrastructure work requested directly. It builds on the switch introduced in #4122._ ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - No product code changed, so there are no unit tests to add. The new logic was verified with purpose-built harnesses instead: - `check-coverage.ps1` run for real against a merged Cobertura report on all three paths — pass, threshold failure, and patch coverage with uncovered changed lines (a scratch commit adding an uncovered method was correctly resolved to `DataTypeException.cs:193`, reported 0 % patch coverage and exited 1). - The gate's accept/reject logic dry-run over `Succeeded` / `SucceededWithIssues` / `Skipped` / empty / `Failed` / `Canceled` — 6/6 correct. - `get-matrix.ps1` exit codes verified in clean processes: empty → 1, normal → 0, `-AllowEmpty` → 0. - All five pipeline/workflow files parse; both PowerShell scripts parse. - Action versions checked against the GitHub API and pinned to current majors. - [x] I have added all necessary documentation. - Rewrote the coverage section of `docs/DeveloperGuide.md` as Required checks and coverage, covering which checks belong in the ruleset and which must not, how coverage is measured, where the numbers appear, and why the two systems disagree. Corrected the stale "BLOCKS" wording in `coverage-thresholds.json`. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - CI YAML, two PowerShell scripts and documentation only; no compilation involved. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - Not applicable — no product code changed. `Opc.Ua.Core.Schema.Tests` was run repeatedly on net10.0 (119 passed) to generate the real Cobertura fragments used to validate the gate. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. - Feedback on the path-relevance diff (now a merge-base diff) and the target-framework numbering in `docs/DeveloperGuide.md` is fixed in `15b8e38df`. - A review pass over the gate/coverage split produced the four correctness fixes listed above, in `863eefc4d`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 26aa55a2-5731-4a4c-b61c-df2f7a785eca	2026-08-07 06:56:05 +02:00
Marc SchierandCopilot	e73e71184d	[WotCon] Add OPC UA WoT Connectivity 1.1 with the Types, source generation, Server and xRegistry work it builds on (#4128 ) ## Summary Adds OPC UA WoT Connectivity 1.1 to the stack, together with the Types, source generation, Server and xRegistry work it is built on. This branch is the merge point for the fourteen-PR stack that replaced integration PR #4093; every constituent PR was reviewed and approved separately and is listed below. 414 files changed, +108,936 / -3,199. Roughly 46k added lines of product code, 57k of tests, plus samples, tools and documentation. ## Types - Complete `ExtensionObject` raw-body decoding for binary, XML and JSON bodies by resolving the concrete type through the message context's encodeable factory, and simplify the `Variant` structure helpers by delegating to it (#4128). - Add `IAtomicFileReplace` as an optional `IFileSystem` capability so existing external implementations keep working, with atomic publish for `LocalFileSystem` via `File.Replace`/`File.Move` and for `VirtualFileSystem` by re-keying the in-memory entry (#4128). - Add lossless conversion between WoT documents and NodeSet2 (`Opc.Ua.Wot.WotNodeSetConverter`). A byte-exact `uav:nodeSet` envelope is preserved when requested, the structured `uav:nodes` projection is used when the readable vocabulary is incomplete, and NodeSet2 is otherwise synthesized from readable WoT terms. Unmapped JSON members survive a round trip as pointer-addressed residue (#4131). ## Source generation - Improve the NodeSet to ModelDesign conversion and the generated node state (#4132). - Generate OPC UA models directly from WoT Thing Description files (#4134). ## Server - Add live NodeManager shadow and immediate reload. A reload materializes a new generation beside the active one and switches atomically; the superseded generation is retired gracefully so existing MonitoredItems keep being served until they drain, or immediately when the caller asks for it. Includes the request admission and drain machinery that lets an orderly shutdown wait for admitted requests instead of tearing down underneath them (#4147). - Make subscription transfer between sessions transactional, so a failed transfer leaves neither session holding a partially moved subscription (#4150). ## xRegistry - Allow clients to use explicit registry roots rather than assuming a well-known location (#4130). ## WoT Connectivity - Add the WoT Connectivity 1.1 information model: a registry-first revision layered on the abstract xRegistry base model, incorporating the complete published OPC 10100-1 v1.02 surface as deprecated nodes in the same namespace so existing 1.02 clients keep working (#4139). - Add the protocol binding abstractions and planners that compile WoT forms into executable plans (#4140). - Add the HTTP, Modbus, OPC UA and MQTT binding executors (#4144). - Add the registry and its stores, including document versioning, validation, dependency resolution and refresh (#4146). - Add the registry client (#4142). - Materialize WoT Thing Descriptions into the server address space, deriving types from Thing Models and instances from Thing Descriptions (#4154). - Restore the WoT asset registry bridge so the deprecated 1.02 asset surface is backed by the registry (#4171). - Add the WoT Connectivity samples and complete the documentation (#4156), and restore the remaining coverage and docs (#4172). ## New projects `src/Opc.Ua.WotCon.Bindings`, `src/Opc.Ua.WotCon.Bindings.Mqtt`, `tests/Opc.Ua.WotCon.Bindings.Tests`, `tests/Opc.Ua.WotCon.Samples.Tests`, and the `samples/WotCon` AggregationServer, AggregationClient and FlatTagServer samples. ## Constituent pull requests Merged into this branch top-down, each independently approved: \| PR \| Title \| \| --- \| --- \| \| #4128 \| [Types] Complete ExtensionObject decoding and add an atomic file replace capability \| \| #4131 \| [Types] Add lossless conversion between WoT documents and NodeSet2 \| \| #4132 \| [SourceGeneration] Improve NodeSet to ModelDesign conversion and generated node state \| \| #4134 \| [SourceGeneration] Generate OPC UA models from WoT Thing Description files \| \| #4147 \| [Server] Add live NodeManager shadow and immediate reload \| \| #4150 \| [Server] Make subscription transfer between sessions transactional \| \| #4130 \| [XRegistry] Allow clients to use explicit registry roots \| \| #4139 \| [WotCon] Add the WoT Connectivity 1.1 information model \| \| #4140 \| [WotCon] Add the WoT protocol binding abstractions and planners \| \| #4144 \| [WotCon] Add the HTTP, Modbus, OPC UA and MQTT binding executors \| \| #4146 \| [WotCon] Add the WoT Connectivity registry and its stores \| \| #4142 \| [WotCon] Add the WoT Connectivity registry client \| \| #4154 \| [WotCon] Materialize WoT Thing Descriptions into the server address space \| \| #4156 \| [WotCon] Add the WoT Connectivity samples and complete the documentation \| \| #4171 \| Restore WoT asset registry bridge \| \| #4172 \| Restore WoT coverage and docs \| Supersedes #4093. ## Follow-up The WoT Connectivity and WoT Binding drafts have moved to 1.1-draft2 since this work was authored. Aligning with that revision - the removed group vocabulary, the new projection/View construct, the remaining model vocabulary terms and the two new portable-identity validation rules - is tracked separately and will follow in its own pull request. ## Validation Built for every target framework with zero warnings and zero errors, and validated at the tip of the stack with `Opc.Ua.WotCon.Tests` at 999 passed / 0 failed and `Opc.Ua.Server.Tests` at 4024 passed / 0 failed. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8	2026-08-05 10:14:15 +02:00
Copilot marcschier Julius Pfrommer	5a690c5a7c	Address WSS discovery review feedback and follow-up tests (#4178 ) # Description Addresses the review feedback for application-aware WSS discovery and incorporates the follow-up coverage from #4179. - Observe the returned task when asserting null `ApplicationConfiguration` validation with `Assert.ThrowsAsync`. - Correct the discovery URL comment grammar. - Verify WSS discovery uses the application's certificate validator by rejecting an untrusted discovery certificate and asserting the TLS authentication failure. - Connect the anonymous WSS binary test through the discovered `ConfiguredEndpoint`, then verify the selected transport profile and security mode. ## Related Issues - Supersedes #4179. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Julius Pfrommer <julius.pfrommer@o6-automation.com>	2026-08-04 21:17:08 +02:00
Marc Schier	00664404c0	Ship the OPC Foundation license file from every package and fix the NU5033 pack failure (#4174 ) # Description The `preview-pack` stage of the Azure DevOps pipeline fails with nine `NU5033` errors, which fails the whole build: ``` NuGet.Build.Tasks.Pack.targets(222,5): error NU5033: Invalid metadata. Cannot specify both a PackageLicenseExpression and a PackageLicenseFile. [src\Opc.Ua.Robotics\Opc.Ua.Robotics.csproj] ``` `common.props` gives every package the shared `LICENSE.txt` as its `<PackageLicenseFile>`, but 19 packages also declared `<PackageLicenseExpression>MIT</PackageLicenseExpression>`. Nine of them hit `NU5033`; the other ten only avoided it by carrying an empty `<PackageLicenseFile></PackageLicenseFile>` workaround. That declaration was wrong on two counts. It advertised the wrong license. `LICENSE.txt` is the OPC Foundation MIT License 1.00, which carries the OPC Foundation copyright and preamble — it is a different text than the plain SPDX `MIT` license. The affected packages therefore published `<license type="expression">MIT</license>` plus a `licenses.nuget.org/MIT` URL on nuget.org, instead of the OPC Foundation text that every other package in the stack ships. It also broke the build, because NuGet refuses to pack a project carrying both a license expression and a license file, failing `pack` solution-wide rather than just for the offending projects. ## Changes The expression is dropped from all 19 packages so every one uses the shared OPC Foundation license file uniformly, like the rest of the stack: * `src/Opc.Ua.OpenUsd`, `.Client`, `.Server` * `src/Opc.Ua.OpenUsdScene`, `.Server` * `src/Opc.Ua.Robotics`, `.Client`, `.Server` * `src/Opc.Ua.Positioning`, `.Client`, `.Server` * `src/Opc.Ua.Di`, `src/Opc.Ua.ISA95`, `src/Opc.Ua.WotCon`, `src/Opc.Ua.Gds.Common` * `tools/Opc.Ua.OpenUsd.Connector.Viewer`, `tools/Opc.Ua.SourceGeneration.Core` * `tools/Opc.Ua.SourceGeneration.Pack`, `tools/Opc.Ua.SourceGeneration.Stack.Pack` — these two set the license not in their own `.csproj` but in the shared `tools/SourceGeneratorPack.targets`, so they were only caught once the new guard ran in CI. The eight per-project empty `<PackageLicenseFile></PackageLicenseFile>` workarounds go away with it. To stop the whole class of failure from recurring, `Directory.Build.targets` now enforces the rule rather than papering over it. Silently clearing the inherited license file would have fixed the build while still publishing the wrong license metadata, so instead a violation fails fast with an actionable error naming the project: ``` error OPCUA0001: Opc.Ua.Foo sets PackageLicenseExpression=MIT. Every package must ship the OPC Foundation MIT License 1.00 via the shared LICENSE.txt that common.props sets as PackageLicenseFile, which the SPDX expression neither matches nor is allowed to coexist with (NU5033). Remove the PackageLicenseExpression property. ``` `common.props` cannot catch this itself: it is imported from `Directory.Build.props`, i.e. before the project body, so it never observes a project-level license expression. `Directory.Build.targets` is imported after the project body and is the earliest place that can. The `common.props` comment that previously hinted the expression was an option now points at this rule. ### Verification * Every packable project evaluates to `PackageLicenseFile=LICENSE.txt` with an empty `PackageLicenseExpression`; file-licensed packages such as `Opc.Ua.Client` are unchanged. * `dotnet pack -c Release` succeeds for `Opc.Ua.Robotics` (previously failing) and `Opc.Ua.SourceGeneration.Core` (previously relying on the removed workaround). * Replicated the CI build commands locally — `dotnet build` of `UA.slnx` (net48/Debug), `tools/SourceGeneration.slnx` (net472/Release) and `tools/MigrationAnalyzer.slnx` (net48/Debug) all succeed with 0 errors. * Inspected the produced `.nupkg`: now `<license type="file">LICENSE.txt</license>` with the license text at the package root, matching the rest of the stack (it was `<license type="expression">MIT</license>` before). * Confirmed the guard fires on a simulated violation (`-p:PackageLicenseExpression=MIT`) and is inert otherwise. ## Related Issues No tracking issue — this fixes the `NU5033` failure in the OPC Foundation Azure DevOps pipeline (build 16247, `preview-pack` stage) and corrects the license metadata published for the affected packages. No product code or public API changes. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. _(Build-infrastructure change with no code surface. Validated by `dotnet pack` of the affected projects, by inspecting the resulting package metadata, and by confirming the new `OPCUA0001` guard fires on a simulated violation. The pipeline `preview-pack` stage is the regression test.)_ - [x] I have added all necessary documentation. _(The rule is documented in place, on the new `Directory.Build.targets` guard and at the `PackageLicenseFile` definition in `common.props`.)_ - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received.	2026-08-04 18:49:50 +00:00
Julius PfrommerandJulius Pfrommer	74e272ee8d	Use application certificate validation during WSS discovery (#4136 ) ConfiguredEndpoint refreshed endpoint descriptions with a DiscoveryClient created only from EndpointConfiguration. That path did not receive the application certificate manager, so WSS discovery could not validate a server certificate trusted by the application before opening a session. Add an ApplicationConfiguration-aware endpoint refresh overload and use it from the default and channel-manager session paths. Preserve the existing overloads for compatibility. The application-aware DiscoveryClient now supplies the configured certificate validation for secure discovery transports while TCP and legacy callers retain their existing behavior. Specification: https://reference.opcfoundation.org/Core/Part6/v105/docs/7.5.3 Tests: UpdateFromServerWithNullApplicationConfigurationThrowsArgumentNullException (1 passed, net10.0) Tests: UpdateBeforeConnectUsesApplicationCertificateValidationAsync (1 passed, net10.0) # Description _Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history._ ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. Co-authored-by: Julius Pfrommer <julius.pfrommer@o6-automation.com>	2026-08-04 10:54:04 +02:00
Julius PfrommerandJulius Pfrommer	da3358d371	Jpfr/fix wss session discovery (#4135 ) # Description WSS binary endpoints use the `opcua+uacp` sub-protocol and carry the normal OPC UA Connection Protocol, SecureChannel, and Session layers. User authentication should therefore behave like `opc.tcp`, independently of whether mutual TLS is enabled for the WebSocket connection. `HttpsServiceHost` previously shared an HTTPS-specific rule with WSS: when `HttpsMutualTls` was disabled, it removed the `Anonymous` user-token policy from the endpoint description. This prevented an anonymous OPC UA session even when anonymous access was explicitly configured by the server. This PR: - Preserves the server’s configured OPC UA user-token policies for WSS UA-SC UA-Binary endpoints. - Retains the existing anonymous-token filtering for HTTPS transports. - Adds an integration test that disables mutual TLS, verifies that the WSS endpoint advertises `Anonymous`, and opens a complete anonymous OPC UA session. - Passes `ApplicationConfiguration` into endpoint discovery performed by `updateBeforeConnect`, allowing WSS discovery to use the application’s certificate manager. - Applies certificate-aware discovery to both the default and channel-manager session paths while preserving the existing endpoint-refresh APIs. ## Verification - Anonymous WSS binary session with `HttpsMutualTls = false`: passed. - WSS `updateBeforeConnect` using application certificate validation: passed. - Null application-configuration validation: passed. - Affected projects build without new warnings. ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. --------- Co-authored-by: Julius Pfrommer <julius.pfrommer@o6-automation.com>	2026-08-04 10:53:40 +02:00
Marc SchierandCopilot	a6a5275006	Add a PumpX-2000 datasheet and diagrams to the pump sample (#4138 ) # Description The `PumpDeviceIntegrationServer` sample simulates OPC 40223 pumps but had no product documentation, a thin nameplate, and a duty point that was not physically self-consistent. This PR gives the sample an official-style product datasheet for its simulated asset and aligns the server to it, so the document and the running address space cannot drift apart. ### New: `samples/PumpDeviceIntegrationServer/DATASHEET.md` A vendor-style datasheet for the fictitious SimPump Corp PumpX-2000 (clearly marked as a simulated device): - Nameplate table mapped to OPC UA browse paths and namespaces (DI / Machinery / Pumps) - Best-efficiency duty point, characteristic curves and a performance table - Operating limits, motor / materials / dimensions data - Instrument list (tag → browse path → unit → `EURange`) and a Mermaid process schematic - Alarm trip points and the deterministic simulation profile ### Server aligned to the datasheet - `PumpDatasheet.cs` — the datasheet expressed as compile-time constants. - `MaterialiseNameplate` — materialises the 19 optional identification properties through the generator-emitted `AddXxx(context)` helpers, so each carries the browse name, namespace and DataType declared by the model (21 nameplate fields in total per pump). - `WithIdentification` now assigns the full datasheet nameplate to every simulated unit. Product-level fields are shared; unit-level fields (`SerialNumber`, `AssetId`, `ComponentName`, `Location`, `FabricationNumber`) are derived from the pump number, so `--pumps N` yields a consistent nameplate for each instance. - Simulation rebuilt around the datasheet curves. Volumetric flow is the only independent variable; differential pressure, mass flow, efficiency and shaft power are derived from `H(Q) = 32 − 0.0104·Q²`, `η(Q) = 72·(1 − 0.6·((Q−25)/25)²)` and `P = ρ·g·Q·H/η`, so `P = Δp·Q/η` holds at every tick. Previously every measurement was an independent sine and the sample published 5 kW of shaft power for 0.05 kg/s at 2 bar Δp. - `EURange` values now come from the datasheet; `OverTempAlarm` reports the `BearingTemperature` chain (the correct `SourceNode` for an alarm whose limits are in Kelvin) rather than a boolean; `Cavitation` and `MotorOverheat` are derived from suction level and bearing temperature with hysteresis instead of `tick % n`. ### Diagrams The sample README gains a simulated-device summary, a link to the datasheet, and three Mermaid diagrams: address space, startup / hosting sequence, and simulation + alarm dataflow. ### Drift protection `PumpDatasheetConformanceTests` pins the nameplate, engineering ranges, trip points, published-value envelope and hydraulic consistency against the datasheet, and the exact pump node-surface baseline in `PumpInstanceNodeIdRegressionTests` covers the new identification properties. No changes to `src/` — this is sample, test and documentation only. ### Rebased on #4117 master has been merged into this branch. #4117 reshaped the same sample (loop-based pump creation via `--pumps N`, `Pump_1` / `Pump_2` BrowseNames with `Pump #n` DisplayNames, historian wiring, `WireBoolean`, per-pump `Diagnostics` group). Both intents are preserved: - the datasheet nameplate is applied through upstream's per-pump `WithIdentification(builder, pump, pumpNumber)` hook, so it scales to `--pumps N`; - `Program.cs` is left exactly as upstream has it — identification is configured centrally, so the old per-pump block there is gone; - the datasheet curve values are published through upstream's status-code / source-timestamp plumbing, and upstream's `WireBoolean` (TrueState/FalseState text + history) is kept; - the node-surface baseline is the union of both identification lists; - `docs/SourceGeneratedNodeManagers.md` was consolidated upstream into `docs/NodeManagers.md`, so the snippet fixes were ported there. ## Related Issues _No tracking issue; this is a documentation and sample-quality improvement to the pump reference sample._ ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. ### Verification (post-merge) - `dotnet build samples/PumpDeviceIntegrationServer` (net10.0): 0 warnings, 0 errors - `dotnet build tests/Opc.Ua.Di.Tests` (net10.0): 0 warnings, 0 errors - `dotnet test tests/Opc.Ua.Di.Tests -f net10.0`: 331/331 passed (includes upstream's new `PumpAddressSpaceComplianceTests` and `DiConformanceFacetTests`) - `dotnet test tests/Opc.Ua.Di.Tests -f net48 --filter Pump`: 44/44 passed - Live client read against the merged sample: Δp 214 079 Pa, ṁ 8.654 kg/s, η 69.33 %, P 2 677.4 W — `Δp·(ṁ/ρ)/η` matches the published shaft power; `Pump_1` reads SN-001 / PMP-1001 / Feed Pump A / Bay 3 and `Pump_2` reads SN-002 / PMP-1002 / Feed Pump B / Bay 4, exactly as documented. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe608993-f3c9-4779-a6b9-9a9eabfc24ba	2026-08-04 10:37:36 +02:00
Marc SchierandCopilot	81d726b7ad	Coordinate the robot cell, and make the twin show what it is actually doing (#4162 ) # Description `MinimalRobotServer` drove its two mobile manipulators through each other. The platforms did clear — R1 and R2 trace figure-eights 1.2 m apart at closest approach, exactly as `README.md` claimed — but each arm reaches 1.611 m (link offsets `0.260 + 0.680 + 0.670 + 0.158` from `robot.usda`), so the arms swept straight through one another while the chassis politely missed. Clearance has to be argued about the deployed arm, not the footprint. Three related gaps sat alongside it: motion was open-loop (two independent sine paths with no awareness of each other), only R1 had a gripper, and nothing was ever actually picked — `PartA`/`PartB` were static children of a single table and the arm replayed a canned eight-pose loop unrelated to where either the robot or the table was. This replaces all of that with a coordinated transfer cycle. ## Keeping them apart The cell is divided into reservable zones, and a robot reserves the zone immediately ahead before entering it, releasing the one behind once clear — block signalling rather than a global schedule: \| Zone \| Extent \| Occupancy \| \| --- \| --- \| --- \| \| `EndZoneA` / `EndZoneB` \| \\|x\\| > 1.3 m \| one robot — a deployed arm needs the whole end \| \| `CorridorEastbound` \| \\|x\\| ≤ 1.3 m, northern lane (y = 0.35) \| one robot \| \| `CorridorWestbound` \| \\|x\\| ≤ 1.3 m, southern lane (y = −1.15) \| one robot \| \| `DockA` / `DockB` \| around each charging dock (y = −2.0) \| one robot \| Three rules follow: - Opposite-direction traffic passes, being in lanes 1.5 m apart; same-direction traffic queues rather than overtaking. - An arm may only leave its transport envelope inside an exclusively reserved end zone. This is the rule that actually fixes the reported defect. - A robot with no work parks on its dock, releasing the end zone so the other robot can deliver into the station it serves. That last rule is not cosmetic. Without it the cell deadlocks permanently on the first handover: an idle robot standing beside its own table blocks the very delivery it is waiting for. A state trace showed R1 blocked at `x = 0.98` forever while R2 sat idle holding `EndZoneB`. The docks are in a southern layby rather than beside the stations, because a robot parked next to a station is close enough to foul one turning into it. ## Where the part actually is A carried part's pose is computed by forward kinematics (`RobotKinematics`) from the same six axis values the server publishes, not from the choreography that produced them — if the arm and the part were computed independently they would visibly drift apart. `RobotArmSolver` solves shoulder and elbow so the tool points down onto a slot, and is checked against that same forward kinematics, landing the tool on every slot of both tables to within 1e-6 m. ## Also simulated Trapezoidal accel/cruise/decel travel with the heading following the path; speed-and-separation slowdown near the other robot; an emergency stop that now halts motion rather than only blinking the beacon; battery drain with a charging detour; seeded grip-slip faults with floor recovery; and KPIs (parts moved, cycle count, last/average cycle time, utilisation, fault count). Both robots now carry a gripper. ## Two hidden couplings this surfaced - `m_r1NodeId` was keyed on "has a tool", so giving R2 a gripper silently moved the dynamic-tool demo and the speed-override command target onto R2. Now keyed on R1 explicitly. - `RobotPositionsDriveLiveOpenUsdSceneAsync` sampled a fixed 1.2 s window and assumed the robot was always moving. Under a duty cycle a robot legitimately stands still while gripping, dwelling or charging, so it now polls until movement is observed — the assertion's intent is that live positions reach the scene, not that the robot never stops. ## Invariants are asserted, not assumed `CellChoreographyTests` runs ten minutes of simulated time at 50 ms and checks on every step that: - oriented footprints never overlap (an oriented box, not a bounding circle — the circle is so pessimistic for a 1.15 × 0.60 m platform that it demands clearances no real cell would need); - the two tool centre points never meet; - a zone is never doubly occupied; - arms only deploy inside an end zone; - parts are conserved — never duplicated, lost or carried twice; - a carried part sits exactly on the tool centre point. Plus the round trip completing, KPIs accumulating, the emergency stop halting every robot, dropped parts being recovered, and the battery draining and recharging. The README documents the zoning, traffic rules and cycle, corrects the misleading clearance claim, and lists ten further realism extensions that were deliberately not built (ISO 10218 / TS 15066 speed-and-separation monitoring, scanner cones, tool changer, inspection station, conveyor infeed, gate interlock, torque telemetry, maintenance counters, time-sampled pose history, traffic priority). ## Making the cycle visible Running the sample end to end against the OpenUSD connector showed the robots driving the cycle and the arms reaching the slots while nothing was picked up or moved. The choreographer tracked part poses and gripper opening purely in memory: the address space only ever exposed the platform poses and the twelve axis angles, so a connector had nothing to subscribe to for the parts and the blocks stayed wherever the asset authored them. A `CellTwin` folder now holds a `ThreeDCartesianCoordinates` position and a `ThreeDOrientation` heading per part, plus a jaw position per robot, bound to `/Cell/Parts/PartNN` and to the gripper jaws. The variables are registered before the motion device system is built - a binding can only carry a `SourceNodeId` that already exists - and sit outside the robotics subtree, because a workpiece is not a device and the robotics builder validates every node below its root as one it allocated itself. The poses are published as structured vectors, not as a matrix. A viewer composes a prim's matrix from the vector translate and rotate ops; the `matrix4d` profile is explicitly left unresolved by the reference connector, which drops a binding it cannot convert in silence. That is the shape the first attempt took, and it failed without a word in any log. The jaws needed the asset restructured. A carrier cube and a finger mesh were siblings under the gripper body, so driving the carrier would have slid it out from under the finger it is bolted to. Each jaw is now one `Xform` owning both, declaring the single matrix op every live-bound prim declares, with the finger points rebased onto the jaw origin. Closed puts the finger faces on the 35 mm half-width of the block, so the gripper grips it rather than passing through it. `RobotAssetContractTests` now holds every live-bound prim to the single-matrix contract, not just the two robot mount points. ## No part starves Collecting the lowest free slot meant a part that never landed in slot 0 was never collected at all: with two parts seeded on `WorkTableA` the second sat untouched for the life of the process while the robots shuttled the other one past it - which reads in the twin as the robots ignoring a block sitting in front of them. Parts are now stamped with the order in which they came to rest and the one that has waited longest is collected, which is what a real cell does with a buffer anyway. Verified end to end: all three parts circulate between both tables, the heading follows the carrying robot and resets on placement, and the jaws stroke between open (0.060) and closed (0.047) as parts are gripped and released. The OpenUSD packages move to 0.3.0-alpha. ## The part was not in the gripper Running it end to end showed a workpiece floating clear of the jaws that were supposed to be holding it. The platform pose and the workpiece pose leave the server as separate bindings published by different loops: positioning at 5 Hz (`UpdateIntervalMilliseconds = 200`), the simulation tick at 20 Hz. A carried part rides the tool centre point, so it was being drawn against a platform up to four ticks stale. Measured against the live scene with the arm stowed — where the part-to-platform distance is fixed by construction — the part sat anywhere between 0.65 m and 0.90 m from its platform instead of at a single distance. A quarter of a metre of drift, three part widths, in front of a 96 mm jaw opening. The platform now publishes at the tick rate, and a carried part is derived from the pose that was actually written to the address space, recorded at the `SetFrameValue` call site rather than where the sample is produced. Each value was individually correct the whole time; only their relationship was wrong, which is why nothing caught it. `RobotOpenUsdE2eTests` now rebuilds the tool centre point from the values authored into the scene and asserts a carried part sits on it. It reproduced the defect at 0.49 m before the fix. The tolerance is the sampling straddle of reading two independently delivered bindings — one publish period is 43 mm at cruise — leaving an order of magnitude between it and the defect. Two assertions in the positioning test wanted the robots a metre apart in X. They pass each other in opposite corridor lanes, where X is all but identical and the separation is entirely in Y; and before the first sample arrives both platforms sit on the zero frame and coincide exactly. They now measure separation in the plane, once both have been seen to move — this was failing intermittently on net48 already. ## The opening view Framing the bounds of an enclosed scene automatically puts the eye inside it: the viewport opened on a close-up of whichever robot happened to be nearest, looking through the fence. The cell now authors `/Cell/OverviewCamera` — the whole floor, both work tables, the docks and the controller cabinet, from 12 m out at 28 degrees of elevation — and the connector opens on the first camera the served root layer declares. It already had the plumbing to honour a camera path; it was simply never given one. `--camera` still overrides the choice outright, and `RobotAssetContractTests` asserts the establishing shot is ordered before `/Cell/TopDownCamera`. ## The mounted gripper was never reported `DynamicToolIsComposedAsync` was failing on both TFMs, in isolation, and a longer timeout did not help. It predates this branch — I confirmed it at `58fd5e8ca` with a clean worktree. It turned out not to be a flaky test at all. Part 5 §9.32.2 lets only a node carrying a `NodeVersion` property trigger a `ModelChangeEvent`, and `AsyncCustomNodeManager` drops entries for nodes that lack one. Neither the mounted tool nor the robot it hangs off had a `NodeVersion`, so every addition was filtered out. Removal kept reporting, because a deleted node is no longer in the manager's index and so takes the "not mine, pass it through" branch of the same filter. That asymmetry is what made it look like flakiness. A connector starting while the tool was mounted composed it and could still deactivate it later; one starting in the six-second gap was never told the tool appeared and never composed it at all, for the life of the process. Which happened depended only on the phase the fixture reached the test in — near-constant, hence a consistent failure rather than an intermittent one. Measured directly before the fix: model-change events arrived only on detach, exactly 18 s apart, and every resolve returned zero components: ``` 14:00:12.738 SERVER mounted <- no event 14:00:24.752 SERVER detached 14:00:24.801 model-change event <- only this ``` After enabling model-change tracking on the robot, the prim tracks the cycle — `A` = composed active (12 s mounted), `i` = inactive (6 s detached): ``` ....AAAAAAAAAAAAAAAAAAAAAAAiiiiiiiiiiiiAAAAAAAAAAAAAAAAAAAAAAAAiiiiiiiiiiii ``` The test now asserts a transition, not a momentary state: the prim seen both active and inactive across more than one full cycle. Unlike the old check this cannot pass on a connector that only ever hears about one direction — which is exactly what the previous pair of tests could not distinguish, since the sibling `DynamicToolIsDeactivatedWhenDetachedAsync` was satisfied by the initial compose and never exercised reconciliation at all. Two robustness gaps in the connector fell out of this: a rejected model-change monitored item was reported as success, leaving every `Dynamic` component frozen at whatever it resolved at start-up; and the initial compose ran before the subscription existed, so a change in between was seen by neither. The first now fails loudly, the second re-resolves once after subscribing. `Opc.Ua.Di.Tests` is now 402/403 on net48 (was 3 failures) and on net10.0, across three consecutive runs of the dynamic-tool tests on each. ## Related Issues - Fixes #4161 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. Verification: full `dotnet build UA.slnx -m:1 /p:UseSharedCompilation=false` — 0 errors, no new warnings. `Opc.Ua.Di.Tests` 399/400 on both net10.0 and net48 (1 pre-existing skip), `Opc.Ua.OpenUsd.Tests` 635, `Opc.Ua.Robotics.Tests` 103. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-08-04 06:58:29 +00:00
Marc Schier Copilot Copilot Autofix powered by AI	724ae71f8a	Make NodeId assignment during a node copy explicit (#4157 ) Fixes #4137. ## The problem `NodeState.Initialize(ISystemContext, NodeState)` copies a node. Every child it materialises through `CreateChild` gets a per-instance NodeId from `ISystemContext.NodeIdFactory`, and is then initialised from its source on the very next statement - which overwrites that NodeId. The assignment is therefore always discarded: it only burns identifiers, and permanently leaks them for factories that track outstanding allocations. Booting the Robotics node manager left ten reservations behind that every later allocation skipped. #4123 fixed the symptom by hiding the factory for the duration of the copy, wrapping the context in one that reports `NodeIdFactory` as `null`. The review objection was fair: a context that lies about one property is hard to reason about, and it only works because that property is allowed to be null. ## What this does States the intent instead of faking the environment. NodeId assignment is now an argument of the two members that materialise children: ```csharp public virtual BaseInstanceState? CreateChild( ISystemContext context, QualifiedName browseName, bool assignInstanceNodeIds = true); protected virtual BaseInstanceState? FindChild( ISystemContext context, QualifiedName browseName, bool createOrReplace, BaseInstanceState? replacement, bool assignInstanceNodeIds = true); ``` A copy passes `assignInstanceNodeIds: false` and every node type - generated or hand-written - sees the real context. There is no capability property, no wrapper, and no second overload to keep in sync. `MethodState`, `BaseDataVariableState` and the source generator template each collapse to a single `FindChild` override that threads the argument into its `CreateOrReplace<Child>` helpers. ## Breaking change The four argument `FindChild` and the two argument `CreateChild` virtuals are gone. - Call sites are unaffected. The new parameter defaults to `true`, so `CreateChild(context, browseName)` and `FindChild(context, browseName, true, null)` keep compiling and keep the 1.5.378 behaviour. - Overrides must add the parameter. A node manager that overrides either member gets `CS0115` until it does. The compiler points at every site; `docs/migrate/2.0.x/node-states.md` shows the before/after. This is deliberate, per review: the stack ships all base types, and types generated from a model are regenerated for 2.0 anyway. ## Also fixed `MethodState.FindChild` returned `OutputArguments` when asked for `InputArguments` with `createOrReplace: false` - a pre-existing bug in the method being restructured. Covered by `FindChildReturnsTheRequestedArgumentsProperty`. ## Validation - Full `dotnet build UA.slnx`: 0 errors, 0 warnings. - `Opc.Ua.Types.Tests`, `Opc.Ua.SourceGeneration.Core.Tests`, `Opc.Ua.SourceGeneration.Tests`, `Opc.Ua.Di.Tests`, `Opc.Ua.Robotics.Tests` - all passing. - Each new regression test was checked to fail without its corresponding fix. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485 Copilot-Session: f45109d9-59cb-4ffd-a4f7-16e1ea449ae4	2026-08-03 20:36:32 +00:00
Marc SchierandCopilot	168b3de353	Replace the RequestManager AsyncLocal guard with an explicit operation context (#4163 ) # Description `RequestManager` tracked whether the calling flow was serving a Client request in a `private readonly AsyncLocal<bool> m_inServiceDispatch`. It was set by a `StandardServer.ProcessRequestAsync` override and read by `NodeManagerLifecycle`, so that a lifecycle operation (`AddAsync` / `ReloadAsync` / `RemoveAsync`) started from inside a request fails fast with `InvalidOperationException` instead of draining — and therefore waiting for — its own request. Two problems with that mechanism, raised in review of #4093: 1. An `AsyncLocal<T>` written inside an `async` method never flows back to the caller that awaited it, so every scope had to chain to the previous value by hand and restore it on dispose. Subtle, and easy to get wrong when a new entry point is added. 2. Ambient state is invisible at the call site, so the lifetime of the mark could not be reasoned about from the code that depended on it — especially once work is handed to a background task. `RequestManager` also had no business carrying ambient state on behalf of a different subsystem; its job is tracking requests for the drain. ## What changed The guard is now an exact identity lookup against the request registry `RequestManager` already maintains, and the operation is threaded explicitly: * `RequestManager` — `internal bool IsExecutingRequest(IOperationContext?)` performs a reference-identity lookup against `m_requests` under the existing `m_requestsLock`. `m_inServiceDispatch`, `EnterServiceDispatchScope()`, the nested `ServiceDispatchScope` class and the flag-based `IsExecutingRequest` property are gone. * `StandardServer` — the `ProcessRequestAsync` override is removed; it existed only to open the dispatch scope. * `INodeManagerLifecycle` — every member (`AddAsync` ×2 factory kinds, `ReloadAsync` ×2, `RemoveAsync`) now takes the caller's `IOperationContext?` directly, ahead of `CancellationToken ct = default`. No overloads and no parallel signature are kept: the interface is new in 2.0 and outside the 1.5.378 compatibility rule, so call sites state their context explicitly or pass `null`. * `HostedNodeManagerLifecycle` and `RuntimeNodeSetLifecycleExtensions` thread the argument through; `AddRuntimeNodeSetAsync` / `ReloadRuntimeNodeSetAsync` remain single methods. * `SystemContextOperationExtensions.GetOperationContext(this ISystemContext)` (new, `Opc.Ua.Core`) lets a NodeManager or Method callback hand over the operation it already received without downcasting. It switches on `SessionSystemContext` and `SystemContext`, the two independent roots every context derives from. No new interface is introduced: casting the context to `IOperationContext` would not do, because a context implements that by delegating to the operation it was created for, so the cast yields the context rather than the operation. * `NodeManagerLifecycle.EnsureNotRequestCallback` no longer consults `ServerState`. The previous `CurrentState == Running` predicate could let a genuinely re-entrant call slip past while the server was shutting down; the request registry answers exactly in every state. A NodeManager or Method callback now reads: ```csharp await m_lifecycle.ReloadAsync(m_registration, replacement, context.GetOperationContext(), ct); ``` and a control-plane caller (hosted service, DI consumer) passes `null`. ## Behaviour Unchanged for the case the guard exists to catch: a lifecycle call made on behalf of an executing request is still rejected up front with the same `InvalidOperationException` and the same message. Two deliberate differences: * Improvement — an internal `OperationContext` that was never enrolled as a Client request is now correctly allowed through. The boolean flag could not distinguish it from a real request. * Accepted trade-off — a caller inside a request that passes no operation is no longer detected at entry and falls back to the pre-existing bounded `RequestManager.RequestDrainTimeout`, which already covered any request that bypassed the service pipeline. Automatic detection for an uncooperative caller inherently requires ambient state, which is exactly what this change removes. Documented in `docs/NodeManagers.md`. ## Alternatives considered and rejected \| Alternative \| Why rejected \| \| --- \| --- \| \| Delete the guard, rely only on `RequestDrainTimeout` \| Turns a clear immediate error into a multi-minute hang. \| \| Infer the caller from the `CancellationToken` it passes \| Works only for the exact request token, breaks on linked tokens; ambient magic of a different kind. \| \| In-flight callback counter on `MasterNodeManager` \| False positives: rejects legitimate control-plane calls whenever any request is concurrently dispatching. \| \| Defer the drain to a background task so re-entrancy cannot deadlock \| The drains are woven into transactional commit / rollback and binding reconciliation; deferring them breaks the transactional guarantees. \| \| `Activity.Current` / `ThreadLocal` \| The same ambient magic the issue asks to remove. \| ## Testing * `RequestManagerTests` — the three `EnterServiceDispatchScope` tests are replaced by coverage for `IsExecutingRequest(context)`: null, never-registered, only-while-scope-open, visible across an `await` and a background `Task.Run`, and two concurrently executing requests not confusing each other. * `NodeManagerLifecycleTests` — rejection from an executing request for Add / Reload / Remove without invoking the factory, acceptance for a context that is not an executing request, acceptance once the request has completed, and the full callback shape (`ServerSystemContext.Copy(operation)` → `GetOperationContext()` → rejected). * `HostedNodeManagerLifecycleTests` — forwarding tests for the caller-context argument on all five members. * `tests/Opc.Ua.Core.Tests/Stack/State/SystemContextOperationExtensionsTests.cs` (new) — `GetOperationContext` across both context roots and the null-argument guard. ## Related Issues - Fixes #4149 ## 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 14402d11-e540-4faa-a5cd-94881bd7ce85	2026-08-03 20:31:51 +00:00
Marc SchierandCopilot	0f257c05f0	OpenUSD scene: replace object in the USD value model with a UsdValue union (#4160 ) Fixes #4151. ## The problem ```csharp public object? Value { get; set; } public SortedList<double, object?> TimeSamples { get; } public IDictionary<string, object?> Metadata { get; } ``` `object` in public API, and every consumer forced into type tests and casts. It was also why `UsdSceneExporter` had to call `Variant.AsBoxedObject(Variant.BoxingBehavior.Legacy)`, itself against guidance. `Variant` cannot stand in for it. The USD value model is recursive and ragged: tuples (`float3`), arrays, arrays of tuples (`color3f[]`), matrices authored as a tuple of row tuples, plus asset paths and prim path references that must round-trip as their own syntax. `Variant`/`MatrixOf<T>` cover rectangular scalar arrays but not the nested "array of tuple" shape. ## What this does Introduces `UsdValue`, a readonly struct scoping a value to the shapes a `.usda` document can express, and converts the model to it end to end - reader, writer, coercion and signature all speak `UsdValue`, so there is one representation rather than conversions at the edges. - `INullable`, so absence is `UsdValue.Null` / `.IsNull` - never `UsdValue?`. - `TryGet` accessors only. No boxing accessor, nothing returning `object`. - `ArrayOf<UsdValue>` for the recursive tuple/array/matrix cases. - `Tuple` and `Array` stay distinct kinds because USD prints `(1, 2, 3)` and `[1, 2, 3]` differently, as do `Token` and `String`. - `Decoerce` now takes the `Variant` directly and reads it through its typed accessors, which is what removes the `AsBoxedObject` call. The attribute's `TypeName` stays authoritative for rendering, so the kinds add type safety without changing a single byte of emitted `.usda`. That is what makes this provably lossless: the existing fidelity and round-trip suites pass with their expectations untouched. ## Three defects the tests caught Worth calling out, because each would have shipped silently: 1. Georeference dual authoring* read values through an `object`-typed `TryToDouble`, which stopped seeing CLR primitives once values became `UsdValue`. 2. A `double3` scalar was emitted as `[1.0, 2.0, 3.0]` instead of `(1.0, 2.0, 3.0)`, because the writer picked brackets from the value's kind. The shape is decided by the TypeName, not the container - so `UsdVal` now renders a composite as a parenthesised tuple unless the type is an array type. This is the same contract the H-1 regression guard documents. 3. `UsdValue` did not load at all on .NET Framework. Storing components as `ArrayOf<UsdValue>` made the struct layout recursive (`UsdValue` -> `ArrayOf<UsdValue>` -> `ReadOnlyMemory<UsdValue>`); CoreCLR resolves that, the net48 type loader throws `TypeLoadException` on every use. Only the net48 run surfaced it - net10.0 was entirely green. The payload is now a plain array, a reference type, which breaks the cycle while the public accessors still hand out `ArrayOf<UsdValue>`. ## Review feedback addressed - `UsdValue.GetHashCode` folds the dictionary entries (an order-independent sum of key/value hashes) instead of only the entry count, so same-size dictionaries no longer all collide. - `UsdValue.ToString` renders a `Dictionary` as its entries ordered by key instead of falling through to the empty-string default, which the metadata fallback paths rely on. - A `uint64` above `long.MaxValue` is no longer cast to `long` (which wrapped `ulong.MaxValue` into `-1`) - it is preserved as its invariant decimal text for scalars, arrays and matrices alike. `TryAsUInt64` reads that form back, and `UsdaReader` now parses an over-long integral literal as a token holding its exact digits instead of throwing `OverflowException`, so the value survives the whole round trip. ## Validation - Full `dotnet build UA.slnx -m:1 /p:UseSharedCompilation=false`: 0 errors, 0 warnings. - `Opc.Ua.OpenUsd.Tests`: 772 passing on net10.0 and on net48 (632 existing + 140 new tests), with 99.0% patch coverage over the changed source lines. - `Opc.Ua.Di.Tests` 364, `Opc.Ua.Robotics.Tests` 103 - unchanged. - No `object`/`object?` left in the `Opc.Ua.OpenUsdScene` public API except the `Equals(object?)` override; no `AsBoxedObject`/`AsBoxedValue` anywhere in the OpenUSD assemblies. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485 Copilot-Session: d089bd1c-b795-4f2b-a872-eb3090c11536	2026-08-03 18:46:42 +00:00
Marc Schier	baab5a9750	Watch datasets that appear after the PubSub application starts (#4164 ) ### Problem `MetaDataPublisher` takes its per-dataset subscriptions once, from `StartAsync`, by walking the connections that exist at that moment: ```csharp m_registry.MetaDataChanged += OnMetaDataChanged; m_subscribed = true; ... SubscribeToDataSets(); ``` An application that is configured after it starts therefore never subscribes to anything — at start there are no writers to walk, and nothing looks again when they arrive. The same gap applies to a dataset added to an already-configured application. The visible effect is that `PublishedDataSet.MetaDataChanged` is never observed for those datasets. A source whose fields are only known once data has flowed — one fed by a live subscription — announces its empty metadata and never corrects it, so a consumer that relies on the announcement to decode the payload never sees a usable one. This is not hypothetical: it is how a host that starts with an empty `PubSubConfigurationDataType` and calls `ReplaceConfigurationAsync` later behaves, which is a reasonable way to drive the application when the configuration is not known at construction time. ### Change `SubscribeToDataSets()` now also runs on `IPubSubApplication.ConfigurationChanged`. Running it more than once means it has to be idempotent, so a dataset that is already being watched is skipped. The comparison is by reference, because that is the identity the handler is bound to — two datasets can carry equal metadata and still be different sources. The new subscription is removed again on dispose alongside the registry handler. The re-entrancy check is done twice under the lock: once before attaching the handler to avoid the work, and once after, because the handler is attached outside the lock and a concurrent configuration change could have added the same dataset in between. ### Scope This does not on its own change the order in which the first announcement and the first data message are published. It only ensures the announcement happens at all for datasets the publisher was never watching. Relates to #4152, which is about that ordering; this is a prerequisite, not a fix for it. ### Verification - `Opc.Ua.PubSub` builds with 0 warnings, 0 errors across all six target frameworks (net10.0, net9.0, net8.0, netstandard2.1, net48, net472). - `Opc.Ua.PubSub.Tests` on net10.0: 1341 passed, 1 skipped, 0 failed.	2026-08-03 19:48:56 +02:00
Marc SchierandCopilot	3713641479	Throttle client publish worker on undeliverable publish responses (+ CodeQL OOM fix) (#4121 ) # Description Fixes a busy-spin in the client publish worker that crashed the net48 CI test host (e.g. [build 16092](https://opcfoundation.visualstudio.com/opcua-netstandard/_build/results?buildId=16092&view=results), `Test Release` reported "The active test run was aborted. Reason: Test host process crashed" on `SubscriptionManagerTests.PublishWorkerDefersDeletingUnknownSubscriptionWhileCreationIsPendingAsync`). It also carries a small, self-contained CI fix for the repository-wide CodeQL out-of-memory failure (see the last section). ## Root cause In `SubscriptionManager.PublishWorkerAsync`, a publish response whose `SubscriptionId` does not resolve to a subscription the manager owns set `moreNotifications = true`. That flag skips the ack-wait throttle, so the worker immediately re-issues `Publish`. The condition is persistent — the server keeps answering with the same undeliverable identifier — so the worker spun with zero delay, logged at `Information` level on every iteration, and in the orphan branch issued `DeleteSubscriptions` for the very same subscription on every single response. With a transport that completes synchronously the loop additionally never released its thread pool thread. Measured on the unmodified code, for a single 485 ms test: \| Metric \| Before \| \|---\|---\| \| `DeleteSubscriptions` calls for one orphan \| 16,500 \| \| "deferred unknown subscription" log lines \| 4,296 \| \| TRX produced by that one test \| 4.86 MB (47% of the whole 2,124-test assembly) \| \| Peak test host working set \| ~528 MB \| On a slow 2-core hosted agent the polling loops stretch out, the spin runs far longer, and the unbounded log/allocation growth takes the test host down. Beyond the CI symptom this is a real client defect: against a live server the client would hammer it with `Publish`/`DeleteSubscriptions` and flood the log whenever a subscription identifier cannot be resolved. ## Changes `src/Opc.Ua.Client/Subscription/SubscriptionManager.cs` - Undeliverable publish responses (retired identifier, identifier still pending creation, or genuine orphan) now always throttle through a new `DelayUnresolvedSubscriptionAsync` — an escalating, bounded backoff (20 ms doubling to a 250 ms cap) — instead of republishing immediately. - After deleting an orphan, its identifier is passed to `RetireSubscriptionId`, so responses already in flight (or emitted before the delete takes effect) are ignored rather than triggering another delete. One delete instead of thousands. - The deferral message is logged once per run of the same unresolved identifier instead of once per response. - The unresolved-response counters reset as soon as a response is dispatched successfully. `tests/Opc.Ua.Client.Tests/Subscription/Fakes/FakeSubscriptionManagerContext.cs` - The call recordings were plain `List<T>` appended to from publish-worker threads while the test thread read them. They are now guarded by a `System.Threading.Lock` and exposed as snapshot `IReadOnlyList<T>` getters. - Added `DeleteCallsCount`, a locked count accessor, so polling wait conditions do not allocate a snapshot per iteration. `tests/Opc.Ua.Client.Tests/Subscription/SubscriptionManagerTests.cs` - `PublishWorkerThrottlesWhileSubscriptionIdIsUnresolvedAsync` — bounds the publish count over a 500 ms window; fails loudly on the old code. - `PublishWorkerDeletesUnknownSubscriptionOnlyOnceAsync` — asserts exactly one `DeleteSubscriptions` call for an orphan. ## Results \| Metric (single test) \| Before \| After \| \|---\|---\|---\| \| Deferred log lines \| 4,296 \| 3 \| \| `DeleteSubscriptions` log lines \| 16,500 \| 3 \| \| TRX size \| 4,856,749 B \| 6,466 B (750× smaller) \| The full-assembly TRX dropped from 10.30 MB to 4.92 MB. No public API change; behaviour change is limited to the undeliverable-publish-response path. ## CodeQL out-of-memory fix (`.github/workflows/codeql-analysis.yml`) The CodeQL job has been failing repository wide — on `master` and on every open PR — with: ``` CodeQL is out of memory. Try running CodeQL on a larger runner (hosted or self-hosted). Error running analysis for csharp: ... codeql database interpret-results --threads=4 ... Exit code was 99 ``` The CodeQL step builds `UA.slnx` without `CustomTestTarget`, so `targets.props` falls through to its default and multi-targets up to six frameworks (`net472`, `net48`, `netstandard2.1`, `net8.0`, `net9.0`, `net10.0`). CodeQL therefore extracts every source file once per framework. The same multiplier applies to source-generated code, which `EmitCompilerGeneratedFiles` materializes on disk — `Opc.Ua.Core.Types` alone emits 52 MB of generated C# for a single framework. The database and source archive grew until `interpret-results` exhausted the runner's memory. - Pin the CodeQL build to `-p:CustomTestTarget=net10.0` so every source file is extracted once instead of ~6×. Verified locally: `UA.slnx` builds with 0 errors under that property. - Limit the analyze step to `threads: 2`, bounding the peak memory of the step that failed. This is unrelated to the publish-worker fix and is happy to be split out if maintainers prefer. ## Review feedback addressed - Snapshot properties are read once into a local before asserting, so two reads cannot observe different snapshots. - Wait conditions poll the new non-allocating `DeleteCallsCount` instead of `DeleteCalls.Count`. - The sync root is declared as the fully qualified `System.Threading.Lock`. An `object`-typed sync root was not used: repository coding standards forbid it, and `Opc.Ua.Types/Polyfills/System.Threading.cs` already backports a public `System.Threading.Lock` for every TFM below net9.0. The branch was rebased onto current `master` (`8a10139b8`). ## Related Issues - No tracked issue; reported via the failing `Test Release` (net48) leg of build 16092. ## 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. _(Internal behaviour fix, no API or documented-behaviour change.)_ - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. _(0 warnings, 0 errors on net48 and net10.0.)_ - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - `Opc.Ua.Client.Tests` net48 (CI filter): 2,124 passed, 0 failed - `Opc.Ua.Client.Tests` net10.0: 2,121 passed, 0 failed - `Opc.Ua.Subscriptions.Tests` net10.0 (integration): 602 passed, 0 failed - `SubscriptionManagerTests` fixture run 5× on net48: 36/36 every run (no flakiness) - [x] I have addressed all PR feedback received. - [x] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6615d151-e4af-43c7-8a32-4531842e7461	2026-08-03 09:50:41 +02:00
Marc SchierandCopilot	ae241b6625	Robotics: the OPC 40010 companion SDK, its client and the robot sample (#4127 ) Stack 5/5 — base #4126. Part of the #4096 split; see #4096 for the overall picture. Closes the API work for #3827. > Review after #4123, #4124, #4125 and #4126. This branch is content-identical to the original #4096 branch, so merging the stack lands exactly what #4096 contained. ## The problem this solves OPC 40010 1.02 defines a rich robot topology but no motion verbs at all — its actuation surface is the SystemOperation and TaskControl state machines plus a Controller `Programs` directory. Meanwhile [URML](https://github.com/URML-MARS/URML) names OPC UA Robotics as its canonical non-ROS substrate and asks for `MoveTo` / `Grasp` / `Release` methods the companion spec does not define, so today it needs a hand-maintained per-deployment NodeId mapping file. This closes that gap in two clearly separated tiers, and never presents the second as standard. ## Contents Server — stock `RoboticsNodeManager`, ordered `IRoboticsModelProvider` composition, `IRoboticsConfigurator` code-behind, and validated fluent topology builders for systems, controllers, motion devices, axes, power trains, motors, gears, drives, safety states, software, task controls and task modules, with the correct OPC 40010 semantic references. Standard operations — `AddSystemOperation` and `AddTaskControlOperation` drive the Part 16 state machines. An illegal cause is rejected with `BadInvalidState` before the handler runs; a handler returning a bad `ServiceResult` does not move the machine; and a transition already in flight rejects a concurrent one rather than letting both commit. Programs — binds the optional `FileDirectoryType` through the reusable `IFileDirectoryBinder` added in 1/5. Operation conventions — opt-in and explicitly non-normative `MoveTo` / `MoveJ` / `MoveL` / `Grasp` / `Release` / `PickFrom` / `PlaceAt` / `SwapTool` / `SetOutput` / `CallProgram`, created only in an application-owned namespace — the OPC UA, DI, IA and Robotics namespaces are rejected — plus a generic `AddOperation<TRequest, TResponse>` extension point. Client — `RoboticsClient` extends the DI client, is subtype aware throughout, and provides typed topology snapshots, the standard operation and task-control clients, `ProgramsAsync`, streaming observation and a DI factory. Verbs resolve by BrowseName, so no NodeId mapping file is needed. Sample — `MinimalRobotServer` composes five models (Robotics, IA, DI, OpenUSD, RSL/GPOS) purely through `IRoboticsModelProvider` contributions, which is the real proof that the stock hosting is sufficient. It follows the published KUKA KR 16-2 kinematics and axis limits with an eased pick-and-place cycle. Also restores the vendor-subtype scanner test held back from 2/5, which uses the Robotics NodeSet as its fixture. ## Defects found and fixed along the way - `ControllerType.CurrentUser` is Mandatory in OPC 40010 but was neither built nor validated. Now has a builder and a validation error. - `MotionDeviceType.TaskControlReference` was absent because it must target a `TaskControlOperationType` instance that did not exist. Now wired. ## Validation `dotnet build UA.slnx` — 0 errors, 0 warnings. \| Suite \| Result \| \| --- \| --- \| \| Opc.Ua.Robotics.Tests \| 103 / 0 \| \| Opc.Ua.OpenUsd.Tests \| 635 / 0 \| \| Opc.Ua.Di.Tests \| 342 / 0 \| \| Opc.Ua.SourceGeneration.Tests \| 94 / 0 \| \| Opc.Ua.Aot.Tests (NativeAOT) \| 120 / 0 \| --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-08-01 07:55:58 +02:00
Marc SchierandCopilot	fb160a8f39	OpenUSD: the OPC UA to OpenUSD binding, scene materialization and connector (#4126 ) Stack 4/5 — base #4125. Part of the #4096 split; see #4096 for the overall picture. > Review after #4123, #4124 and #4125. ## What this is The generic OPC UA to OpenUSD binding, scene materialization, and the connector that drives a live stage from a running server. ## Contents Binding (Part 1) — `Opc.Ua.OpenUsd` and `Opc.Ua.OpenUsd.Server`. A server describes how its Variables drive USD prims, and serves the USD layers it refers to as Part 5 files with a content digest. Scene materialization (Part 2) — `Opc.Ua.OpenUsdScene` and `Opc.Ua.OpenUsdScene.Server`, including the USDA writer. - Escaping fix: string values, composition-arc references, variant set names and selections, `doc` and prim names were all emitted unescaped, so a value carrying a quote or a newline could author its own layer syntax. Every emit site now goes through the escapers. The tests assert the right property — not that the payload is absent (it legitimately survives inside the escaped literal) but that no payload becomes its own line of layer syntax. Client — `Opc.Ua.OpenUsd.Client` streams live values into an `IUsdSink`. `CompositeUsdSink` fans out to several sinks; the file sink interpolates between server samples so motion is continuous rather than stepped at the publishing interval. Connector — `tools/Opc.Ua.OpenUsd.Connector` composes a stage from a live server. The optional in-process viewport lives in a separate assembly probed at run time, so the connector still runs — and says why — when it is absent. - Fix: the asset cache and PKI root moved off the shared temp directory to `LocalApplicationData`. Sample — `PumpDeviceIntegrationServer` gains an OpenUSD representation of the pump. ## Notes for the reviewer - The viewport assembly is net10.0-only. It is what exercises the `Directory.Build.targets` change from 1/5: without it, the `net8.0` leg of the per-TFM CI build fails with `CS0433` because the `System.Threading.Lock` polyfill collides with the real type. - `UsdAssetDelivery` hashes through `SHA256.Create` rather than the static `HashData`, which does not exist on the net472/net48 targets this test project also builds for. ## Validation `dotnet build UA.slnx` — 0 errors, 0 warnings. \| Suite \| Result \| \| --- \| --- \| \| Opc.Ua.OpenUsd.Tests \| 621 / 0 \| \| Opc.Ua.Di.Tests \| 321 / 0 \| --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-08-01 07:55:58 +02:00
Marc SchierandCopilot	c48c8b93dd	Positioning: OPC 10000-210 RSL and 10000-211 GPOS on one geolocation provider (#4125 ) Stack 3/5 — base #4124. Part of the #4096 split; see #4096 for the overall picture. > Review after #4123 and #4124. ## What this is The OPC 10000-210 Relative Spatial Location and OPC 10000-211 Global Positioning companion models, with their server and client libraries, plus the ISA-95 migration onto the shared geolocation seam. ## Why it is one change A deployment writes one provider implementation and both OPC 10000-211 and OPC 10030 are served from it. The specifications disagree on representation, not on content: - GPOS publishes the structured `GlobalLocationDataType` and validates the EPSG code against the configured CRS. - ISA-95 publishes the same sample as the `String` array that OPC 10030 §8.3.2.2 requires. Splitting them would mean two provider interfaces for one physical sensor, which is exactly what this avoids. The seam itself lives in `Opc.Ua.Server` and landed in 1/5. ## Contents - `Opc.Ua.Positioning`, `.Server` and `.Client`. - `Opc.Ua.ISA95.Server` moves onto `IGeoLocationProvider`; the three ISA-95-specific provider files are removed because the shared seam and its in-memory implementation replace them. - Fixes a conformance gap: the ISA-95 binder wrote a scalar where the type declares `ValueRank` OneOrMoreDimensions. It now writes `String[]`. - Fixes a liveness bug: a provider that cannot push is polled at the Variable's `MinimumSamplingInterval` rather than installed once, so a non-pushing provider no longer serves its start-up sample with `Good` status forever. - `samples/MinimalIsa95Server`. ## Validation `dotnet build UA.slnx` — 0 errors. \| Suite \| Result \| \| --- \| --- \| \| Opc.Ua.Positioning.Tests \| 69 / 0 \| \| Opc.Ua.ISA95.Tests \| 136 / 0 \| \| Opc.Ua.Di.Tests \| 304 / 0 \| --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-08-01 07:55:58 +02:00
Marc SchierandCopilot	0357380335	Source generator: typed NodeSet method signatures and referenced model accessors (#4124 ) Stack 2/5 — base #4123. Part of the #4096 split; see #4096 for the overall picture. > Review after #4123. The diff shown against `master` will include #4123 until that merges. ## What this is Source generator only; no runtime library changes. ## Contents - Typed NodeSet method signatures. A method imported from a NodeSet now gets a typed signature. Methods that share a declared signature are grouped onto one generated `MethodType`, named after its owner when the plain name would collide, so two unrelated `Execute` methods no longer share a class. - An override that keeps the signature it inherits keeps the declaration it overrides. Pointing `MethodDeclarationNode` at the override itself made `MethodDeclarationId` self-referential. OPC 10000-4 lets a caller invoke a method with the NodeId of its declaration in the ObjectType or any supertype, and `MethodState.MethodDeclarationId` is what the server matches on, so `Call` stopped resolving and returned `Bad_MethodInvalid` for every alarm subtype. An override that genuinely changes its signature still becomes its own declaration. - `StateNumbers` / `TransitionNumbers` were emitted empty for every NodeSet-sourced model, because such children arrive as `VariableDesign` rather than `PropertyDesign` and were filtered out. Any companion model built from a NodeSet was writing `StateNumber = 0` on the wire — a Part 16 defect well beyond the models in this stack. The ModelDesign path is unchanged. - Referenced models can contribute fluent accessors, carried by a model dependency payload so a downstream model can extend an upstream one. ## Notes for the reviewer - `tests/Opc.Ua.SourceGeneration.Tests` is left at its `master` content here. Its new vendor-subtype test uses the Robotics NodeSet as its fixture, so it can only land with the Robotics change at the top of the stack (5/5). - `PumpInstanceNodeIdRegressionTests` gains six `InputArguments`/`OutputArguments` entries for argument-less standard condition methods. Typed method signatures materialise those properties from the `ConditionType` declaration even where the method takes none. That is a real deviation, pinned rather than hidden so it stays visible; it is worth a follow-up. ## Validation `dotnet build UA.slnx` — 0 errors. \| Suite \| Result \| \| --- \| --- \| \| Opc.Ua.SourceGeneration.Core.Tests \| 3739 / 0 \| \| Opc.Ua.SourceGeneration.Tests \| 81 / 0 \| \| Opc.Ua.Di.Tests \| 304 / 0 \| \| Opc.Ua.History.Tests \| 506 / 0 \| \| Opc.Ua.Gds.Tests \| 1095 / 0 \| History and GDS are listed because they are what the method-declaration fix above is load-bearing for: without it they fail with 10 and 370 errors respectively. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-08-01 07:55:57 +02:00
Marc SchierandCopilot	ea0f266b66	Move build and test to the Azure DevOps managed pool and replace Codecov with an in-pipeline coverage gate (#4122 ) # Description Moves the all-target-framework build, the cross-platform test matrix and the Native AoT run from GitHub Actions onto the Azure DevOps `netstandard` Managed DevOps Pool, and replaces Codecov with a coverage gate that runs inside the pipeline. ## 1. CI backend switch One conceptual switch decides who owns that work. It is checked into source in two places that must be flipped together: \| File \| Setting \| Default \| \| --- \| --- \| --- \| \| `azure-pipelines.yml` \| `parameters.ciBuildBackend` \| `ado` \| \| `.github/workflows/buildandtest.yml` \| `env.CI_BUILD_BACKEND` \| `ado` \| Setting both to `actions` restores the previous behaviour exactly. Azure Pipelines gains what it was missing to take the work over: - A Linux all-TFM `sln.yml` leg, replacing `build-all-tfm-linux`. `sln.yml` gets a `configurations` parameter so this leg builds `Release` only instead of doubling the managed-pool job count. - A Linux agent in the `net10.0` `testfastpr` matrix, replacing the ubuntu `build-and-test` job. - Native AoT and Code Coverage now gate pull requests instead of running only on Schedule/Manual. The coverage stage drops its `testdebug` dependency, which is Schedule/Manual-only and would otherwise have skipped coverage on every PR. - On a PR, Windows builds the .NET Framework TFMs and Linux the modern ones — mirroring the split the GitHub Actions jobs used — so all seven TFMs stay covered without fanning every TFM across both pools. Two things are deliberately not covered by the switch: - macOS stays on GitHub-hosted runners, because Managed DevOps Pools provide no macOS image. - *`master378` and `develop/` keep running the GitHub Actions jobs, since Azure Pipelines only builds `master`/`main` from this file. GitHub Actions does not expose the `env` context to job-level `if:` conditions, so the switch is resolved once in the existing `discover` job and re-exported as job outputs. A new `build-and-test summary`** job gives branch protection one stable check to require, instead of the generated matrix job names. It treats an intentionally skipped job as success but still fails on a real failure or cancellation. The `paths:` allow-list was moved off the `pull_request` trigger and into `discover`. A workflow filtered out by `paths` never reports its checks, and a required check that never reports leaves a PR permanently "Expected — waiting for status to be reported". That would have made the routine "ratchet the coverage floor" PR (which touches only `coverage-thresholds.json` and docs) unmergeable. ## 2. Coverage gate replacing Codecov Codecov is removed entirely — `codecov.yml`, both uploads, the badge, and the token usage. `.azurepipelines/check-coverage.ps1` runs at the end of the `Code Coverage` stage against the Cobertura report ReportGenerator already merges, driven by `coverage-thresholds.json`: \| Check \| Behaviour \| \| --- \| --- \| \| Project floor \| Blocking — total line and branch rates against an absolute floor. \| \| Patch coverage \| Blocking on PRs — changed lines must reach `patch.target`, tolerating `patch.threshold` percentage points (the same semantics `codecov/patch` had). \| \| Baseline delta \| Advisory — reports the change against the recorded baseline, never fails. \| The project rate is recomputed from the per-line data rather than read off the Cobertura root element, so the `ignore` globs carried over from `codecov.yml` apply to the project floor as well as to the changed-lines gate — otherwise samples and generated code would count towards one gate but not the other. `testcc.yml` gains an explicit `checkout: self` with `fetchDepth: 0`, because the default shallow clone cannot diff against the base branch. ## Follow-up actions for maintainers Two items cannot be automated from source: 1. Enable the `/azp run` gate. In the Azure DevOps portal: pipeline → More actions → Triggers → Pull request validation → Require a team member's comment before building a pull request, scoped to Only on pull requests from non-team members. This makes Copilot-authored and outside-contributor PRs wait for a maintainer's `/azp run`, mirroring the approval GitHub Actions already requires. The documentation added here describes the workflow and the private-org-membership gotcha; the setting itself still has to be switched on. 2. Update branch protection and secrets. Drop the `codecov/project` and `codecov/patch` required checks, require `build-and-test summary` and the ADO coverage check instead, and remove `CODECOV_TOKEN` from the GitHub repository secrets and the ADO variable group. Also note the thresholds in `coverage-thresholds.json` are seeded from the last Codecov reading (73.61 %) with headroom, because the in-pipeline number had not been observed yet. The first full `Code Coverage` run on master should confirm it and the floors should be tightened then — the gate prints the exact figure it computes. ## Related Issues _No tracking issue; this is CI infrastructure work requested directly._ ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - No product code changed, so there are no unit tests to add. The new logic was verified with purpose-built harnesses instead: - All 8 branch/event/backend combinations of the `discover` switch, confirming `master378` and `develop/` are untouched and that `actions` restores the old behaviour. - 12 scenarios for `check-coverage.ps1`: the project floor, the patch gate, ignore globs applying to both* gates, three path-normalization shapes, and the advisory delta. - 14 path-relevance cases confirming the in-job filter matches the retired `paths:` allow-list exactly. - End-to-end against a real ReportGenerator report: correctly resolved `src/Opc.Ua.PubSub/DependencyInjection/PubSubBuilder.cs`, found exactly the four modified tracked lines and reported 50 % patch coverage (2 covered, 2 uncovered). - [x] I have added all necessary documentation. - New Continuous integration section in `docs/DeveloperGuide.md`, a CI note in `CONTRIBUTING.md`, and links from `docs/README.md`. Also corrected the CI matrix description, which claimed the PR gate ran net48 + net8.0 — it runs net48 + net10.0. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - `dotnet build src/Opc.Ua.Server` → 0 warnings, 0 errors. No product code changed. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - Not applicable — no product code changed. `Opc.Ua.Server.Tests` was run on net10.0 during development (3480 passed; the single `ServerFluentApiHostingTests` failure is pre-existing and reproduces on master). - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. - A review pass over this branch flagged the `paths:`-filter deadlock described above; it is fixed in `1926e240e`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 181ba7ea-c72f-42d7-a737-00a3faadaa17 Copilot-Session: 26aa55a2-5731-4a4c-b61c-df2f7a785eca	2026-07-31 21:06:25 +00:00
Marc SchierandCopilot	76d33cb139	Remove SourceGenerator.Foundations, ship analyzers under Roslyn-versioned folders for VS2022 and VS2026, and validate the packages (#4120 ) # Description The two OPC UA source generators were built on `SourceGenerator.Foundations` (SGF), whose only job was getting the generator's dependency closure loaded inside the compiler. It does so with a process-wide, first-generator-wins `AppDomain.AssemblyResolve` hook backed by a non-concurrent `Dictionary` mutated from `AssemblyLoad` callbacks. Concurrent compilations in a shared `VBCSCompiler` race it, which is the root cause of the intermittent clean-build failure: ``` CSC : error CS8784: Generator 'StackSourceGeneratorHoist' failed to initialize. ... Exception was of type 'FileNotFoundException' with message 'Could not load file or assembly 'SourceGenerator.Foundations.Contracts, Version=2.0.14.0, ...' ``` The repository already solves that problem deterministically — #4041 made the NuGet packages ship the full closure under `analyzers/dotnet/.../cs`, and `Directory.Build.targets` registers the same closure as `Analyzer` items for in-repo `ProjectReference` consumers. SGF was therefore pure liability and is removed. ## What changed 1. SourceGenerator.Foundations removed. `ModelSourceGenerator` and `StackSourceGenerator` are plain `[Generator(LanguageNames.CSharp)] IIncrementalGenerator` implementations again. SGF's implicit crash handling is replaced by an explicit `SourceGenerator.Guard` that reports `MODELGEN003` / `STACKGEN003` and rethrows cancellation. Its logger sink was already dead code and is dropped. `Opc.Ua.SourceGeneration.Stack.dll` shrank from 18 MB (embedded closure) to 29 KB. 2. Analyzers now ship under Roslyn-versioned folders. `analyzers/dotnet/roslyn4.14/cs` and `analyzers/dotnet/roslyn5.0/cs` instead of `analyzers/dotnet/cs`, driven by `roslyn.props` at the repository root, with new `.Pack` projects owning the package identity. The versioned folder matters: a host older than the band skips* the analyzer instead of loading it and failing inside generator initialization. 3. Dependencies flow automatically. Both delivery paths discover the payload by globbing the generator's output directory, so adding a package to a generator's dependency tree requires no `.csproj`, `.targets`, `.nuspec` or validation-script edit. Verified by temporarily adding a `PackageReference` and watching it appear in the payload with no other file touched. ## Two Roslyn bands: VS2022 and VS2026 Analyzers ship twice, once per band. The .NET SDK loads the highest band its compiler supports and silently skips anything above it, so a single-band package leaves every consumer below that band with no diagnostics and no generated code. \| Band \| Built against \| Loaded by \| \| --- \| --- \| --- \| \| `roslyn4.14` \| `Microsoft.CodeAnalysis.CSharp 4.14.0` \| Visual Studio 2022 17.14+ / .NET 9 SDK \| \| `roslyn5.0` \| `Microsoft.CodeAnalysis.CSharp 5.0.0` \| Visual Studio 2026 18.0+ / .NET 10 SDK \| Getting this wrong is invisible, which is most of why it took three attempts. The rule: > An analyzer must reference the lowest `System.Collections.Immutable` / `System.Reflection.Metadata` across all hosts it claims to support, and must never ship them. .NET binds a reference upward but never downward. Every failure mode is a compiler warning: \| Diagnostic \| Cause \| \| --- \| --- \| \| `CS9057` \| Built against a newer Roslyn than the host, so it is skipped entirely \| \| `CS8784` \| Loaded, then `MissingMethodException` - a shipped S.C.I bound a second `ImmutableArray<T>` \| \| `CS8032` \| Analyzer instance could not be created \| This PR first shipped a `roslyn4.8` band that never loaded - confirmed against a real Roslyn 4.8 compiler: ``` warning CS8784: Generator 'ModelSourceGenerator' failed to initialize ... MissingMethodException: Method not found: 'IncrementalValueProvider`1<ImmutableArray`1<!!0>> IncrementalValueProviderExtensions.Collect(IncrementalValuesProvider`1<!!0>)' ``` That band was removed, and this is not a regression: master had the same defect, pinned to Roslyn 4.14 while referencing `System.Collections.Immutable 10.0.0.0`, so no 4.x host ever worked there either. The band that landed instead is `4.14`, and it is nearly free: `Microsoft.CodeAnalysis 4.14` depends on the same S.C.I 9.0.0 as 5.0.0 and its `csc` runs on `net9.0`. Both bands therefore share `$(RoslynRuntimeVersion)`, so `Opc.Ua.Types` and `Opc.Ua.SourceGeneration.Core` are compiled once and serve both; only the five Roslyn-referencing assemblies get a second build, each from a 12-line project that glob-links the primary sources and overrides `RoslynApiVersion`. Going lower still - 4.8 wants S.C.I 7.x - would mean rebuilding that whole closure, `Opc.Ua.Types` included, and remains out of scope. (`System.Collections.Immutable 7.0.0` also does not resolve in this repository: `NU1603`, 8.0.0 substituted.) Nothing else in the repository exercises the down-level payload, and since every failure is a warning, a broken band would ship silently - exactly as the 4.8 one did. So `validate-source-generator-packages.ps1` now restores a real `Microsoft.Net.Compilers.Toolset 4.14.0`, runs its `csc.dll` over the packed `roslyn4.14` payload, and asserts both that none of `CS9057`/`CS8784`/`CS8032`/`CS8034` is emitted and that `/reportanalyzer` lists the generator as having executed - silence alone would also pass if the generator were never handed to the compiler. The migration analyzer, whose payload comes from a hand-written nuspec rather than `SourceGeneratorPack.targets`, gets the same check plus a band/assembly manifest assertion. To stop the trap recurring, `Microsoft.CodeAnalysis`, `System.Collections.Immutable` and `System.Reflection.Metadata` are excluded from both the package payload and the in-repo analyzer closure, and asserted against in the validation script. `roslyn.props` and `docs/DeveloperGuide.md` document what adding a band requires. ## The shipped package set is unchanged Evaluating `PackageId`/`IsPackable` across every project in `preview-pack.slnx` gives the same 47 package ids* on `master` and on this branch, and a full Release build plus `dotnet pack --no-build` produces exactly those 47. Only the owning project changed: \| Package \| master \| this PR \| \| --- \| --- \| --- \| \| `…Opc.Ua.SourceGeneration` \| `Opc.Ua.SourceGeneration.csproj` \| `Opc.Ua.SourceGeneration.Pack.csproj` \| \| `…Opc.Ua.SourceGeneration.Stack` \| `Opc.Ua.SourceGeneration.Stack.csproj` \| `Opc.Ua.SourceGeneration.Stack.Pack.csproj` \| `.azurepipelines/expected-packages.txt` pins the expected set and the validation script fails when the packed output drifts. ## Two new package checks `build/<PackageId>.props` naming. NuGet only auto-imports that exact name, and the file declares every `CompilerVisibleProperty` / `CompilerVisibleItemMetadata` the model generator relies on. It was previously packed as `build/OPCFoundation.Opc.Ua.SourceGeneration.props`, which does not match the package id — so no NuGet consumer ever imported it, and every `ModelSourceGenerator` property and `AdditionalFiles` metadata was silently invisible. In-repo projects were unaffected because they import it by relative path. An end-to-end consumer that actually drives the generator.* The pre-existing clean-consumer check only proved the analyzer loads. `Test-SourceGeneratingConsumer` builds a standalone project that feeds a real NodeSet2 in as an `AdditionalFile`, pins a `ModelSourceGeneratorPrefix` that cannot be derived from the model, and references the emitted types from hand-written code. It deliberately does not import the props by path, so the package's auto-import is what is under test. Both checks were verified to fail against a package rebuilt with the misnamed props. ## CI: parallelism re-enabled `buildandtest.yml` pinned three `dotnet build` invocations to `/p:UseSharedCompilation=false` and `-maxcpucount:1`. `dotnet build` defaults to all logical processors, so `-maxcpucount:1` actively disabled parallelism. Both flags were introduced silently in unrelated PRs (`f358078b1`, `208c601b0`) and suppress exactly the conditions the SGF race needed — now structurally gone. Compilation is ~72 % of what that workflow executes (233 of 323 job-minutes in run `29916768976`), so the queue-independent `Build` step durations are the baseline: \| Measurement \| Baseline \| \| --- \| --- \| \| `Build` step, 26 `test-` jobs \| mean 5.53 min, sum 143.9 min \| \| `build-linux-all-tfm` \| 56.3 min \| \| `build-windows-all-tfm` \| 32.5 min \| `codeql-analysis.yml` keeps the flag — the CodeQL tracer requires it. ## Incidental fixes - `Microsoft.CodeAnalysis.CSharp` declares an exact `[version]` range on `Microsoft.CodeAnalysis.Common`, so with `CentralPackageTransitivePinningEnabled` the central pins must move in lockstep or restore fails with `NU1109`. They now derive from `$(RoslynApiVersionVS2026)`. - `MigrationGeneratorTests` created its generator driver without parse options — silently fine under Roslyn 4.14, `Inconsistent language versions` under 5.0. - The migration analyzer props injected `<Analyzer>` items by hand from `analyzers/dotnet/cs`; NuGet registers analyzers itself and selects the matching folder, so the injection was obsolete and would now force the wrong assembly to load. - Exception diagnostics reported `ex.StackTrace` (null for an exception that never left its throw site, and carrying neither type nor inner chain) — now `ex.ToString()` at all three call sites. - The pack guard tested the flattened assembly list, so it only fired when no* variant produced output; it now checks each declared variant contributed assemblies. - `GeneratePackageOnBuild` is disabled for the generators; packing happens in CI or on an explicit `dotnet pack`. ## Note for reviewers Building this repository requires a Roslyn 5.x host (the .NET 10 SDK or Visual Studio 2026). That is already the documented prerequisite and every CI leg installs `10.0.x` last, but it is now a hard requirement. ## Related Issues _No tracking issue: this is a follow-up to the CS8784 clean-build failure reported against #4041, which fixed the packaged delivery path but left the in-repo project-reference path relying on the SGF resolver._ - Follow-up to #4041 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. ### Verification detail \| Check \| Result \| \| --- \| --- \| \| `dotnet build UA.slnx` (clean, and after merging master) \| 0 errors; 18 warnings, all pre-existing `CA1873` in `Opc.Ua.Client` \| \| `Opc.Ua.SourceGeneration.Tests` \| 81/81 on net10.0 and net48 \| \| `Opc.Ua.SourceGeneration.Stack.Tests` \| 92/92 on net10.0, 89/89 on net48 \| \| `Opc.Ua.SourceGeneration.Core.Tests` \| 3722 passed, 8 skipped \| \| `Opc.Ua.MigrationAnalyzer.Tests` \| 121/121 on net10.0 and net48 \| \| Package id set, master vs branch \| identical, 47 packages \| \| Release build + `dotnet pack --no-build` \| 47 packages, matching the manifest; both `roslyn4.14` and `roslyn5.0` present; no host-provided assemblies shipped \| \| `validate-source-generator-packages.ps1` \| passes, including manifest, props-naming, end-to-end generator consumer and down-level host checks \| \| Down-level band under real csc 4.14.0 \| all three analyzer packages load and run \| \| Negative tests \| misnamed props → both checks fail as designed; hidden variant output → pack guard fires; 5.0 build placed in `roslyn4.14` → real `CS9057`; non-running generator name → execution assertion fires; expecting a `roslyn4.8` band → manifest check fires \| --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9ed53262-5cfa-429a-820e-bf6146a00d8b	2026-07-31 15:46:17 +00:00
Marc SchierandCopilot	f0ab7878ea	Core stack: reusable file directory binder, geolocation seam and state machines (#4123 ) Stack 1/5 — base `master`. Part of the #4096 split; see #4096 for the overall picture. ## What this is The core stack changes the Robotics, OpenUSD and Positioning work depends on. No new projects; every change is to an existing one, so this is reviewable without any of the companion specs. ## Contents `Opc.Ua.Server` - `FileSystem` gains a reusable `IFileDirectoryBinder` and `IFileSystemHost`, so any node manager can serve a Part 5 `FileDirectoryType` over `IFileSystemProvider` instead of that binding living inside one companion model. - Entry names are validated before they reach the provider. The contract promises forward-slash provider-relative paths but never enforced it, so a backslash payload such as `..\..\..\Windows\System32\evil.txt` bypassed the obvious provider-side check on Windows. Defence in depth — the shipped `PhysicalFileSystemProvider` was already safe. - `Location` adds a single `IGeoLocationProvider` seam so one provider implementation serves both OPC 10000-211 GPOS and OPC 10030 ISA-95. The two specs disagree on representation, not on content, so the difference is a projection rather than a reason to write the provider twice. Ships a WKT text formatter and an in-memory provider. - `StateMachines` adds `FiniteStateMachineDispatcher`, the Part 16 transition guard used by DI software update and later by Robotics task control. `Opc.Ua.Types` - Node copies no longer consume NodeIds from the `NodeIdFactory`. `Initialize` materialises children through `CreateChild`, which assigns a per-instance NodeId, but the copy then overwrites every child NodeId from its source, so the assignment is discarded. A counting factory silently burned identifiers; a tracking one leaked them. - `JsonDecoder` resolves a relative encodeable type id against the namespace table before the factory lookup. `Opc.Ua.Client` — a file system path segment written without a namespace prefix is qualified with the namespace of the parent it is resolved against, matching the provider-owned BrowseNames. `Directory.Build.targets` — the `RestrictForLegacyTfm` no-op now also applies when a project pins a single `TargetFramework` and `CustomTestTarget` names a different one. That is the same situation it already handled for the legacy frameworks, and it is what keeps the per-TFM CI matrix green once net10.0-only projects appear later in the stack. ## Validation `dotnet build UA.slnx` — 0 errors. \| Suite \| Result \| \| --- \| --- \| \| Opc.Ua.Types.Tests \| 8088 / 0 \| \| Opc.Ua.Server.Tests \| 3873 / 1 \| \| Opc.Ua.Client.Tests \| 2105 / 0 \| \| Opc.Ua.Core.Tests \| 4145 / 0 \| \| Opc.Ua.Features.Tests \| 134 / 0 \| The single `Opc.Ua.Server.Tests` failure is `ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, which is pre-existing and environmental: the PKI path in my working directory exceeds MAX_PATH. It reproduces on clean `master` and passes from a shorter directory. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-07-31 13:13:47 +02:00
Marc SchierandCopilot	8ce1bf1899	Preserve publish queue timestamp ordering (#4016 ) # Description Fixes publish request assignment ordering in `SessionPublishQueue`, following up on the review of the proposed #3997 follow-up. Two related problems were identified during review: 1. Retrying an already-ready Subscription from `PublishTimerExpired()` reset its `Timestamp` when no Publish request was available, which broke oldest-first selection among equal-priority Subscriptions. Master already uses a single lock for the ready check and the Publish-request enqueue, so the original lost-wakeup window does not require removing the `ReadyToPublish` guard. 2. `PublishTimerExpired()` bypassed the selection policy used by `PublishAsync()`. It iterated `m_queuedSubscriptions` — a `ConcurrentDictionary` with no ordering guarantee — and handed each newly notifying Subscription straight to the first waiting request, so `Priority` and `Timestamp` were ignored whenever several Subscriptions became ready in the same timer tick. This PR now: - Keeps the `ReadyToPublish` timer early exit so already-ready Subscriptions retain their timestamps. - Flags all notifying Subscriptions as available first and then drains the waiting requests through `GetSubscriptionToPublish()`, so requests are served highest priority and longest waiting first regardless of dictionary iteration order. - Routes `PublishCompleted(..., moreNotifications: true)` through the same path instead of assigning the Subscription directly. - Replaces `AssignSubscriptionToRequest()` with `AssignSubscriptionsToRequests()` / `TryAssignSubscriptionToRequest()`. The latter also skips a request whose task completed (cancelled or timed out) between the `IsCompleted` check and `TrySetResult`, instead of losing the Subscription. - Adds deterministic regression coverage for both behaviours. ## Related Issues - Follow-up to #3997 - Review correction to #4017 - Tracks the equivalent `master378` correction in #4119 ## Testing - All 26 `SessionPublishQueueTests` pass on net10.0 and net48. - `PublishTimerAssignsWaitingRequestToHighestPrioritySubscriptionAsync` and `PublishTimerPreservesReadySubscriptionTimestampOrderAsync` both fail without the corresponding source change. - The full `Opc.Ua.Server.Tests` project shows no new failures on net10.0; the single `ServerFluentApiHostingTests.ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync` failure reproduces unchanged on the branch without these edits. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 181ba7ea-c72f-42d7-a737-00a3faadaa17	2026-07-31 11:56:50 +02:00
Julius PfrommerandJulius Pfrommer	2fb20c06a2	Use standardized OPC UA WebSocket transport profile URIs (#4133 ) The WSS transport constants used the unregistered uawss- prefix for the UA-SecureConversation/UA-Binary and UA-JSON profiles. Use the profile URIs registered by the OPC Foundation: https://profiles.opcfoundation.org/profile/254 https://profiles.opcfoundation.org/profile/253 The WebSocket mappings and sub-protocols are specified in OPC UA Part 6 section 7.5.2: https://reference.opcfoundation.org/Core/Part6/v105/docs/7.5.2 Update the transport-profile helpers, regression tests, and documentation accordingly so endpoint discovery publishes identifiers recognized by conforming OPC UA clients and servers. Tests: dotnet test tests/Opc.Ua.Core.Tests/Opc.Ua.Core.Tests.csproj -f net10.0 --filter FullyQualifiedName~TransportProfileHelpersTests (39 passed) # Description _Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history._ ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. Co-authored-by: Julius Pfrommer <julius.pfrommer@o6-automation.com>	2026-07-31 09:39:11 +02:00
Marc SchierandCopilot	8a10139b8c	Fix OPC UA address-space compliance in the device integration stack (#4117 ) # Description A live OPC UA compliance audit of `samples/PumpDeviceIntegrationServer` against the OPC 40223 Pumps and OPC 10000-100 (DI) companion specifications found 13 address-space violations plus a serialization bug in the MCP tool. Every fix lands at its root cause in the shared stack rather than as a workaround in the sample, so all servers benefit. The sample now exposes N fully simulated pumps (`--pumps N`) and advertises the DI conformance facets it actually satisfies. > This branch has been merged with `master` including #4116 ("Add simulation to PumpDeviceIntegrationServer and Event modelling"), which independently reworked the same pump simulation and the same fluent alarm builders. See [Relationship to #4116](#relationship-to-4116) below for how the two were reconciled. ## Findings and fixes \| # \| Finding \| Root cause \| Fix \| \|---\|---------\|-----------\|-----\| \| 1 \| `OverTempAlarm` created by the fluent API did not exist in the address space \| `AttachAlarm` attached the alarm with `AddChild` but never indexed it into the node manager \| Register the node; the same gap existed in `AddObject`, `CreateInstance` and the state-machine creators \| \| 2 \| Alarms were unsubscribable — `EventNotifier = 0` everywhere, no `HasNotifier` from `Server` \| No event-source wiring at all \| Set `HasCondition`, initialise `SourceNode`/`SourceName`/`ConditionName`/`InputNode`, promote `EventNotifier` up the ancestor chain, publish `HasNotifier` from the `Server` object \| \| 3 \| Every instance NodeId was minted in the DI standard namespace (`ns=4;s=5001_Pump #1`) \| `DiNodeManager.New` inherited the parent's namespace index \| New `DiNodeManager.InstanceNamespaceIndex`; instances move to the server's application namespace \| \| 4 \| `Server/Namespaces` described only 2 of 6 namespaces \| The NodeSet2 importer drops nodes parented to namespace 0, so companion-spec metadata objects never reached the address space — and no route covered the server's own namespace \| New `NamespaceMetadataPublisher` walks the `NamespaceArray` and fills version/publication date from `ModelDependencyAttribute` \| \| 5 \| Vendor BrowseNames (`Diagnostics`, `LastError`, …) sat in the DI namespace \| Builders defaulted to the parent's namespace \| Default to the server namespace; spec-defined BrowseNames are unchanged \| \| 6 \| `AccessLevel="5"` from the Pumps NodeSet was reported as `1` \| The ModelDesign `AccessLevel` enum is not `[Flags]` and cannot represent `CurrentRead \\| HistoryRead` \| Carry the verbatim NodeSet2 bitmask and emit named `AccessLevels` constants \| \| 7 \| `Historizing = true` while `HistoryRead` returned `BadHistoryOperationUnsupported` \| Nothing reconciled the declared history surface with the wired historians \| Startup clears the advertisement (including masking the attribute read callbacks) when no provider resolves; the sample wires a real historian \| \| 8 \| Mandatory `ProductInstanceUri` unset \| Sample omission \| Populated on every pump \| \| 9 \| Mandatory `TrueState`/`FalseState` empty on every supervision boolean \| Absent in the official NodeSet; a server must still populate them \| Populated on every pump \| \| 10 \| Machinery `Machines` folder empty (OPC 40001-1 §9.2 says shall) \| `TryAddToMachinesFolder` used `AddChild`, which re-parents the device away from `DeviceSet` \| Add the `Organizes` reference instead \| \| 11 \| BrowseName contained a space and `#` \| Sample naming \| `Pump_1` with the readable label in `DisplayName` \| \| 12 \| `Maintenance` group materialised but empty \| Sample omission \| Populated \| \| 13 \| Uninitialised values reported `Good` \| No initial-value status \| `BadWaitingForInitialData` until the first simulation tick \| \| — \| `ServerProfileArray` advertised only `StandardUA2017` \| `DiNodeManager` did not implement the existing `IConformanceContributor` \| Contribute DI 1.05 server facets computed at runtime from what is actually wired, merged with `StandardUA2017` \| \| — \| MCP tool serialised boolean `false` and numeric `0` as JSON `null` \| `Variant.Null` is `default`, so `Variant.Equals` adopts the non-null type and compares against the default value \| Dispatch on `BuiltInType` and use typed `TryGetValue` accessors instead of the prohibited `AsBoxedObject` \| ## Relationship to #4116 #4116 landed on `master` while this branch was in flight and targeted overlapping ground. The merge is a union of both intents, not a fast-forward: - `AlarmBuilderExtensions` — kept both sides. #4116 contributes the parent-must-be-an-Object guard, `SetEnableState`, and the `HasEventSource` references; this branch contributes the `HasCondition` reference, condition source initialisation, registration of the alarm with the node manager so it is browsable, notifier promotion up the whole ancestor chain, and root-notifier registration. - `SupervisionBuilderExtensions` — took #4116's `SetAlarmActive`. It is more spec-correct than this branch's version: it honours `EnabledState`, resets Acked/Confirmed on activation, and computes `Retain` per OPC 10000-9 so an unacknowledged Condition stays retained after going inactive. This branch's null guard was kept so a malformed alarm cannot dereference a missing `EnabledState`. - `PumpNodeManager.Configure.cs` — took #4116's push-based simulation (`RegisterPumpSimulation`, `CreatePumpSimulation`, `IValueUpdater<T>`, `Initialize`/`Advance`/`Publish`, deterministic per-pump phase offsets) as the structure, then folded in this branch's compliance work: configurable pump count, Maintenance group, `TrueState`/`FalseState`, historian wiring, per-pump mandatory Identification values, and `BadWaitingForInitialData` until the first tick. - README — union, keeping #4116's address-space validation workflow section. Two test adjustments were needed, both because the merge legitimately changed behaviour the tests pinned: - `PumpHostedReferenceTests` now resolves pump NodeIds from the browse result instead of hard-coding the DI namespace, and waits for the first published value. `PumpCreatedAfterStartupJoinsTheLiveSimulationAsync` was added to restore #4116's coverage that a pump created after startup through `ConfigureDevicesFor` joins the live simulation. - `FluentAlarmRegistrationIntegrationTests` no longer asserts that `Retain` clears when an alarm goes inactive — that pinned this branch's weaker semantics, and #4116's Part 9 behaviour is correct. ## Breaking change Runtime-created DI device instance NodeIds move from the companion-spec namespace to the server's application namespace, e.g. `ns=<DI>;s=5001_Pump #1` becomes `ns=<application>;s=5001_Pump #1`. This invalidates NodeIds cached by clients from earlier 2.0 previews. It is not a model change — standard DI/Machinery/Pumps type NodeIds and spec-defined BrowseNames are untouched. Device integration hosting is new in 2.0, so there is nothing to migrate from 1.5.378 and this is deliberately not in `docs/MigrationGuide.md`. Anyone tracking 2.0 previews should rediscover devices by browsing `DeviceSet` rather than persisting NodeIds, and resolve namespace indexes from `NamespaceArray` per connection. ## Verification Compliance is pinned by regression tests that assert through the server's service surface (`BrowseAsync` / `ReadAsync` / `HistoryReadAsync`), not by inspecting `NodeState` objects in memory. That distinction mattered: three defects survived in-memory assertions and were only caught through the read path — most subtly, `Historizing` was cleared while `AccessLevel` still advertised `HistoryRead`, because attribute read callbacks re-applied the NodeSet bits. Post-merge on net10.0: \| Suite \| Result \| \|---\|---\| \| `Opc.Ua.Di.Tests` \| 326 passed \| \| `Opc.Ua.Server.Tests` \| 3846 passed, 5 skipped \| \| `Opc.Ua.Tools.Tests` \| 288 passed \| \| `Opc.Ua.History.Tests` \| 506 passed, 27 skipped \| \| `Opc.Ua.SourceGeneration.Core.Tests` \| 3722 passed, 8 skipped \| The full solution was also built and tested on net48 before the merge. Review feedback from `copilot-pull-request-reviewer` has been addressed in commit `badad31dc`: the shared `ServerObjectState` lock was removed (the forward `HasNotifier` edge is published through the owning node manager instead), MCP array serialization now preserves element types, and the pump option guards report the specific rejected option. The only failure is `ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a pre-existing certificate-store issue that passes when run in isolation and is unrelated to this change. ## Documentation Updated `docs/DeviceIntegration.md`, `docs/MigrationGuide.md`, `docs/HistoricalAccess.md`, `docs/DeveloperGuide.md`, `docs/McpServer.md`, `docs/README.md`, `samples/PumpDeviceIntegrationServer/README.md` and `tools/Opc.Ua.Mcp/README.md`, and consolidated the four node-manager documents into a single [`docs/NodeManagers.md`](docs/NodeManagers.md) — overview, built-in managers (including `MasterNodeManager`), core vs custom, registration, server address-space metadata, and source generation — with a TOC and all inbound links repointed. ## Related Issues No tracking issue exists for this work yet — it originated from an ad-hoc compliance audit of the pump sample. Given the size of the change and the runtime-breaking NodeId move, please open (or link) a tracking issue so the design is recorded as an ADR before merging. - Fixes # ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d50c3fc-58b1-4c99-85a4-1e32c88666d9	2026-07-30 16:04:18 +02:00
Marc SchierandCopilot	636912bd4c	Harden subscription retirement after #4114 and refresh the client namespace table on model change (#4115 ) # Description Two things, both surfaced by the live NodeManager registration work in #4094. > Rebased on #4114. Upstream landed #4114 (`cc4667d0b`) while this PR was open; it fixes the > same #4113 root cause. `origin/master` is merged in and the conflicts were resolved in favour of > the upstream API shape — `CompleteAsync(IMessageProcessor, uint subscriptionId, ct)` with the > last known server id passed as an argument. The `IMessageProcessor.LastServerId` member this > branch had added is dropped again. What remains here is the orthogonal hardening #4114 does not > cover, plus all of the #4100 work. ## 1. Remaining subscription-retirement hardening (on top of #4114) #4114 fixed the id-collision itself. Three related holes in the same paths are still open: - Retirement was not atomic with removal. `RetireSubscriptionId` is now called under `m_subscriptionLock` from both `CompleteAsync` and `RemoveSecondaryPartitionAsync`. `GetById` resolves incoming publish responses under the same lock, so a worker could otherwise observe the subscription as removed while its id had not yet reached the retired-id ring — and send a redundant `DeleteSubscriptions` for an already-retired id. `RemoveSecondaryPartitionAsync` did not record the retired id at all. - The retired-id ring was far too small. `kMaxSubscriptionHistory` 10 → 256, so subscription churn cannot evict live history. - The orphan delete was unguarded. The publish worker no longer sends `DeleteSubscriptions` for an unresolved id while any subscription is still awaiting its id. A publish response can overtake the `CreateSubscription` continuation that assigns the id, and deleting in that window kills a healthy subscription. Leaving a genuine orphan alive until its lifetime expires is the safe trade-off. - `PublishControllerAsync` lost resize signals. It re-created its `AsyncAutoResetEvent` waiter on every loop iteration; the abandoned waiter stayed enqueued and swallowed a later `Set()`, so the publish worker pool stopped resizing. This is a second, latent starvation path independent of the id collision. The wait is now created once and only re-armed after it completes. ## 2. Managed client does not refresh the namespace table on model change (#4100) `NodeManagerLifecycle.NotifyCommittedChangeAsync` updates `NamespaceArray`, bumps `UrisVersion`, and then reports a `BaseModelChangeEvent` — an event shape that carries no `Changes` payload. The client tracker did observe it and cleared the `INodeCache`, but it held only an `IStreamingSubscription`, an `INodeCache` and an `ILogger`, so nothing could re-read the namespace table. A tracking client therefore resolved NodeIds from a namespace added by a live NodeManager registration against a stale `NamespaceUris` table. - New `INamespaceTableRefresher` (`NamespaceUris` + `FetchNamespaceTablesAsync`). The member signatures match what `Session` and `ManagedSession` already expose, so both satisfy the interface without any new members. `ISession` is deliberately left untouched — moving its members onto a base interface would be a binary break for 1.5.378 consumers. - `ModelChangeTracker` takes the refresher as an optional trailing constructor argument, which keeps every existing call site source-compatible and makes `null` the opt-out. A refresh runs when the event carries no per-node detail, when an affected node is `Server_NamespaceArray`, or when an affected namespace index is beyond the client's table. It runs before the cache is invalidated and before `ModelChanged` is raised, so subscribers that re-browse from the handler already see the new uris. A failing refresh is logged and swallowed. - `ModelChangedEventArgs.NamespaceTableRefreshed` reports the refresh to applications. - `ManagedSession.EnableModelChangeTrackingAsync` wires the session in, so opting into tracking is all an application has to do — no new option, no change to the positional `CreateAsync` signature. - `ModelChange.TypeDefinition` changes from `NodeId?` to `NodeId`. `NodeId` is a readonly struct implementing `INullable` and must not be wrapped in `System.Nullable<T>`; use `NodeId.Null` and `IsNull` instead. The type is 2.0-preview only (absent from 1.5.378), and the only producer is the tracker itself, which already assigns a non-null `AffectedType`. Per review feedback the type is also now a `readonly record struct`, matching `DataValueChange` / `EventNotification`. ## Tests Every new regression test was run against a temporarily re-introduced defect and fails there: \| Test \| Covers \| \|---\|---\| \| `CompleteAsyncRetiresOnlyTheCompletedSubscriptionWhenOthersArePendingCreationAsync` \| retire-by-identity (also guards #4114's fix) \| \| `CompleteAsyncRemembersRetiredServerIdAfterIdWasResetAsync` \| retired-id history \| \| `PublishWorkerDefersDeletingUnknownSubscriptionWhileCreationIsPendingAsync` \| orphan-delete guard \| \| `PublishControllerKeepsResizingAfterWorkersExitAsync` \| publish-controller signal loss \| \| `ConcurrentAddAndCompleteRetireOnlyCompletedSubscriptionsAsync` \| parallel add/retire registry integrity \| \| `ConcurrentSubscriptionChurnNeverStarvesASubscription` (integration) \| #4113 repro against the reference server \| \| 7 × `ModelChangeTrackerTests` namespace-refresh cases \| each trigger, no-trigger, throwing refresher, `null` refresher, ordering \| \| `LiveNodeManagerAddRefreshesTheClientNamespaceTable` (integration) \| #4100 repro — times out without the wiring \| `FakeMessageAckQueue` records both the completed id (upstream's assertions) and the completing instance (the identity assertion in `MessageProcessorTests`), so neither side's coverage was lost in the merge. Results after merging `origin/master`: - `Opc.Ua.Client.Tests` — 2119/2119 pass on net10.0; the new `ModelChange` and `SubscriptionManager` fixtures also pass on net48. - `Opc.Ua.Subscriptions.Tests` churn + coverage fixtures — 28/28 (net10.0); churn fixture also passes on net48. - `Opc.Ua.Server.Tests` `NodeManagerLifecycle` — 49/49* on net10.0; the new fixture also passes on net48. - No new analyzer warnings; only the three pre-existing `CA1873` hits in `NodeCacheResolver.cs` and `ClassicSubscriptionEngine.cs` (untouched files) remain. ## Documentation - `docs/ModelChangeTracking.md` — new Namespace table refresh section with the trigger table, updated `ModelChangedEventArgs` / `ModelChange` snippets, updated manual-construction snippet, and a quick-reference row. - `docs/NodeManagerRegistration.md` — client-side half of the Namespaces section. No `docs/migrationguide.md` entry: every changed type is either `internal` or 2.0-preview-only. ## Related Issues - Fixes #4100 - Follow-up hardening for #4113 (closed by #4114) - Related to #4094, #3993 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e22f9b3c-1682-40bd-8b72-31a11352fa2c	2026-07-30 12:17:00 +02:00
dpseandMarc Schier	cc4667d0b5	Complete client subscriptions by identity instead of id (#4114 ) # Description Completes subscriptions by passing `IMessageProcessor` and last known server subscription id to prevent incorrect subscription removal when multiple subscriptions with id 0 exist. ## Issue During calls to `SubscriptionManager.CompleteAsync`, the subscription manager looked up the subscription entry to remove by the passed subscription id. But newly created subscriptions not yet having received an id from the server have their id set to 0, meaning that disposing an unrelated subscription also with id set to 0 could remove the wrong one (as there can exist multiple subscriptions with the same zero-id), which in turn could lead to the id no longer being resolved, and the publish worker treating it as unknown. Deleted subscriptions also have their id reset to zero, which could potentially clash as well. ## Fix - Internal interface `IMessageAckQueue` is changed so that `CompleteAsync` also takes `IMessageProcessor` as an argument. - `MessageProcessor` stores the known server id (if set) and passes itself along with that id to `CompleteAsync`. - With the `IMessageProcessor` argument, `SubscriptionManager` no longer needs to look up subscription by id and therefore removes the correct subscription. - Additionally, `SubscriptionManager` receives the non-zeroed server id and no longer enqueues zero-value ids to the subscription history. ## Related Issues - Fixes #4113 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. --------- Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>	2026-07-30 09:33:50 +02:00
Alexander KöpkeandCopilot	4251fec84d	Add simulation to PumpDeviceIntegrationServer and Event modelling (#4116 ) This pull request introduces significant improvements to the Pump Device Integration Server sample, focusing on robust event/alarm integration, simulation enhancements, and comprehensive test coverage. The changes ensure that pump objects and their events are correctly wired for OPC UA eventing, improve alarm state management and reporting, and update documentation and tests to validate these behaviors. Event and Alarm Integration Improvements: * Pump objects and their event sub-objects now have `EventNotifier` flags and are linked via `HasNotifier` references, ensuring they properly participate in the OPC UA event hierarchy. Each pump is also registered as a root notifier. [[1]](diffhunk://#diff-6d01bd352b39d1bd0a96e8c0c11d27c066ae33a64a552bdabe023535ca833fc2R297-R307) [[2]](diffhunk://#diff-6d01bd352b39d1bd0a96e8c0c11d27c066ae33a64a552bdabe023535ca833fc2R256-R257) * The alarm builder now enforces that alarms are attached only to valid object nodes, enables alarms by default, and wires `HasEventSource` references and `EventNotifier` flags for both parent and alarm nodes. [[1]](diffhunk://#diff-2d10b87cac23754d35da8632e9b1a23ec0c1e706689c7241da36dbc934b2666aR208-R215) [[2]](diffhunk://#diff-2d10b87cac23754d35da8632e9b1a23ec0c1e706689c7241da36dbc934b2666aR235-R248) Alarm State and Reporting Enhancements: * Alarm state transitions now check that the alarm is enabled before updating state, set acknowledged/confirmed flags, assign new event IDs, update timestamps, and report events using OPC UA mechanisms. * Alarm severity and message are set dynamically based on the active state, and the `Retain` flag is managed according to OPC UA alarm state requirements. Simulation and API Changes: * `CreatePumpAsync` now registers new pump instances with the simulation, ensuring all pumps participate in the live simulation loop. * Simulation ticks and monitored value publishing are unified and described in the documentation, with deterministic phase offsets for each pump. [[1]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cL98-R134) [[2]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cR178-R193) Documentation Updates: * The README is updated to reflect the new event/alarm integration, simulation details, and instructions for validating the address space with automated and manual workflows. [[1]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cL40-R73) [[2]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cL98-R134) [[3]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cR178-R193) Test Coverage Expansion: * The main integration test is expanded to verify that both pumps publish unique, changing monitored values and that alarm events are reported and observable via OPC UA subscriptions. [[1]](diffhunk://#diff-818e6fe65e5a28c2405bb287d9e40fcb74f1275c90755da56c60343528e17ee5L57-R58) [[2]](diffhunk://#diff-818e6fe65e5a28c2405bb287d9e40fcb74f1275c90755da56c60343528e17ee5R165-R314) These changes ensure the sample is a robust reference for OPC UA eventing, simulation, and alarm integration, and that its correctness is validated both automatically and through enhanced documentation.# Description _Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history._ ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-30 06:05:01 +02:00
Marc SchierandCopilot	0383472ebc	Fix PubSub occurrence publishing and JSON DataValue status encoding (#4106 ) ### Summary Three related gaps mean no `PublishedDataSet` can produce a message that is not a key frame or a derived delta frame. `PubSubDataSetMessageType.Event` and the `ua-event` wire tag of Part 14 §7.2.5.4 are modelled but unreachable. This PR also corrects the JSON `DataValue` status member to the Part 6 Table 42 name `Status`. ### 1. A dataset cannot declare what its sample is `PublishedDataSetSnapshot` carries a metadata version, fields and a timestamp. `WriterGroup.BuildDataSetMessageAsync` therefore decides the message kind alone: `KeyFrameCount <= 1` forces `KeyFrame`, otherwise it derives a delta by comparing the snapshot positionally against the previous one. That derivation assumes successive snapshots describe the same fields in the same order, which holds only for a source returning its complete current state every cycle. A source whose samples are occurrences — an event, a condition snapshot, or a delta it has already computed — cannot satisfy it: consecutive samples carry different fields, so field i of one occurrence is compared against field i of an unrelated earlier one, and `Math.Min` silently truncates when the counts differ. `PublishedDataSetSnapshot` now carries an optional `MessageType`. When set, the writer publishes exactly the supplied fields under the declared type and leaves its key-frame state untouched. When null — the default, and every existing caller — behaviour is unchanged. ### 2. The event writer was unreachable `EventDataSetWriter` and `EventPublishedDataSet` implement the Part 14 §6.2.4 event model correctly, but nothing could use them: - `EventPublishedDataSet` did not implement `IPublishedDataSet`, so `PubSubApplication` could not register an event dataset; - nothing in the library constructed `EventDataSetWriter`, and it was not an `IDataSetWriter`; - `WriterGroup` appends at most one message per writer per cycle, whereas an event writer produces one per occurrence. Now: `EventPublishedDataSet` is an `IPublishedDataSet` whose general `SampleAsync` drains one occurrence per cycle declared as an event; `EventDataSetWriter` is an `IDataSetWriter`; `PubSubApplication` builds an event dataset when the configured `DataSetSource` is a `PublishedEventsDataType` and the registered source is an `IEventSampler`; and a writer group drains every pending occurrence in one publish cycle. Event-writer sequence numbers now use the WriterGroup checkpoint state, so failover and restart resume above the shared checkpoint. The rows-returning `EventPublishedDataSet.SampleAsync` is renamed `SampleEventsAsync`, both to make room for the interface member and to match `IEventSampler.SampleEventsAsync`. ### 3. A metadata announcement hides its own payload `JsonMetaDataMessage` documents that the payload "is exposed both on the base `PubSubNetworkMessage.MetaData` property and on `MetaDataPayload` so callers can use whichever accessor matches their fluent style." `MetaDataPublisher` set only `MetaDataPayload`, so the documented contract was false: any consumer, transform or encoder hook reading the base property saw an announcement as a message carrying nothing. Both are now populated. Dataset subscriptions also use a dispose-safe subscribe/recheck handshake, preventing a concurrent shutdown from leaving a late handler attached. ### 4. JSON DataValue status used the wrong member name Part 6 section 5.4.2.18 Table 42 names the `DataValue` status member `Status`; `StatusCode` is its data type. `JsonEncoder` and `JsonDecoder` instead shared the member name `StatusCode`, so stack-to-stack round-trips passed while standards-conformant peers could not exchange non-Good status values. Both directions now use `Status`. The encoder regression test also verifies that `StatusCode` is absent from the emitted `DataValue` object. Fixes #4111. ### Compatibility The snapshot property is optional and defaults to null, so existing PubSub sources and current-state sampling behavior remain unchanged. The `EventPublishedDataSet.SampleAsync` rename is source-breaking on a type that could not previously be reached from the runtime. The JSON `DataValue` wire member intentionally changes from `StatusCode` to the specification-defined `Status`. No legacy alias is retained: the encoder emits `Status`, and the decoder expects `Status`. ### Testing `Opc.Ua.PubSub.Tests` on `net10.0`: 1341 passed, 1 skipped, 0 failed. New coverage: - `WriterGroupDeclaredMessageTypeTests` — a declared type is honoured, declared samples never become delta frames and are never suppressed as unchanged, and an undeclared source keeps the existing key-frame cadence. - `EventDataSetWriterTests.EventPublishedDataSet_SamplesOneOccurrencePerSnapshotAsync` — the general contract drains one occurrence per call, declares it as an event, and reports nothing when exhausted. - `WriterGroupEventWriterHotStandbyContinuesSequenceNumberFromSharedCheckpointAsync` — event messages share the checkpointed sequence state and continue monotonically after standby promotion. Issue #4111 validation: - `Opc.Ua.Types.Tests` JSON encoder/decoder fixtures on `net10.0`: 274 passed. - `Opc.Ua.Core.Encoders.Tests` JSON conformance cases on `net10.0`: 604 passed. - `Opc.Ua.PubSub.Tests` JSON fixtures on `net10.0`: 88 passed. - Targeted `net48` regression tests: 2 passed; core JSON conformance cases: 572 passed. - Review-feedback fixtures on `net48`: 11 passed. CI-fix validation: - `Opc.Ua.Core.Tests` on `net10.0`: 4145 passed, 86 skipped. - `Opc.Ua.Core.Tests` on `net48`: 4070 passed, 92 skipped. - Patch coverage check passes after adding targeted metadata and event publishing tests. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a300bc8-8bde-4a82-8fc3-476ef837bc6a Copilot-Session: 9f18bf87-795f-40a9-8494-b6e6334c4a97	2026-07-29 22:06:52 +02:00
Marc SchierandCopilot	cfd15f08e1	Assign instance NodeIds in generated CreateOrReplace<Child> helpers (#4104 ) # Description The model source generator emits two families of instance helpers with inconsistent NodeId behaviour: * `ISystemContext.CreateInstanceOf<Type>(parent, browseName)` rebases the whole materialised subtree onto per-instance NodeIds. * `<Owner>.CreateOrReplace<Child>(context, replacement)` — the plumbing behind `NodeState.CreateChild` / `ReplaceChild` — did not. The child it produced was either bare (null NodeId, no browse name) or kept the type-level NodeId of the replacement. Any node manager mixing the two had to hand-roll a recursive re-stamp before registration, or sibling instances collided on NodeIds — a failure that only surfaces at registration time. `samples/PumpDeviceIntegrationServer` did exactly this. ## What changed Generator (`tools/Opc.Ua.SourceGeneration.Core`) * `CreateOrReplace<Child>` gains `bool assignInstanceNodeIds = true`. When enabled and the context supplies an `ISystemContext.NodeIdFactory`, a child whose NodeId is null or still equal to its declaration NodeId — and its descendants — are rebased through `NodeInstanceExtensions.AssignInstanceNodeId` / `AssignInstanceChildNodeIds`, with the owner as reference root. A NodeId the caller already assigned is never overwritten. * The helper also stamps the new child's `SymbolicName` / `BrowseName` / `DisplayName`. Without an identity, a path-based `INodeIdFactory` derives `{parent}_` for every child of a parent, so the assignment would have introduced collisions rather than removed them. `BrowseName` stamping is guarded on `context.NamespaceUris != null` — event-reporting paths pass a context without a namespace table. * All generated `Create` factory call sites pass `assignInstanceNodeIds: false`: they build declaration subtrees whose NodeIds must stay at their type-level values, and the enclosing `CreateInstanceOf<Type>` factory rebases the finished subtree in a single pass. The `CreateInstanceOf<Type>` rebase guard drops the `parent != null` requirement — supplying a `browseName` is what marks a dynamically materialised instance. Calling it without one (as the generated `NodeStateActivator`s do, and when replacing a well-known singleton) still keeps the declaration NodeIds. * `replacement` is now emitted as `BaseInstanceState?` on `CreateOrReplace<Child>` and on the generated `FindChild` override, matching the documented contract and the base `NodeState.FindChild` signature. Hand-written twins — `MethodState.CreateOrReplace{Input,Output}Arguments` and `BaseDataVariableState.CreateOrReplaceEnumStrings` gain the same parameter and behaviour, sharing a new internal `NodeInstanceExtensions.AssignNewChildInstanceNodeIds`. This is also required so the generated `public new` overrides keep hiding a matching base signature (otherwise `CS0109` with `TreatWarningsAsErrors`). Sample — `PumpDeviceIntegrationServer` now passes the real `DeviceSet` parent to the factory and drops `AssignChildNodeIds` plus the manual root stamp. Docs — new "Materialising instances at runtime — NodeId assignment" section in `SourceGeneratedNodeManagers.md` and a refreshed `DeviceIntegration.md`. ## Design note: why not the guard proposed in the issue Gating on "the owner already has an instance NodeId" cannot be implemented as `owner.NodeId != <ownerTypeNodeId>`. The generated child factories call `<Owner>.CreateOrReplace<Child>` on declaration instances — `CreatePumpType_Operational` runs on a `FunctionalGroupState` whose NodeId is `ObjectIds.PumpType_Operational`, not `ObjectTypeIds.FunctionalGroupType` — so the comparison would be true while the type template is being materialised (`forInstance: false`) and would rewrite the address space's declaration NodeIds. The explicit opt-out separates the two callers unambiguously. ## Compatibility Adding an optional parameter is source-compatible. Behaviour changes for callers of `NodeState.CreateChild` / `ReplaceChild` when the context carries a `NodeIdFactory`: the new child now carries a browse name and a per-instance NodeId instead of nothing. ## Related Issues - Fixes #4099 > [!NOTE] > The issue asks to delete `AssignChildNodeIds` from `samples/MinimalRobotServer`. That sample does not exist on `master` — it only lives on the still-open #4096 branch and carries no such helper. The equivalent boilerplate on `master` is `PumpDeviceIntegrationServer/PumpNodeManager`, which this PR cleans up. Details in [this comment](https://github.com/OPCFoundation/UA-.NETStandard/issues/4099#issuecomment-5102275507). > [!NOTE] > One commit is an unrelated one-line-scope build-hygiene fix: two `CA2000` warnings in `ConsoleReferenceClient` where subscription ownership transfers to the session. Happy to split it out if preferred. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. ### Tests added Generator emission (`NodeManagerGeneratorTests`) — `CreateOrReplaceChildAssignsInstanceNodeIdsByDefault`, `TypeFactoriesOptOutOfCreateOrReplaceNodeIdAssignment`, `CreateInstanceOfFactoriesRebaseWithoutAnExplicitParent`. Helper behaviour (`NodeInstanceExtensionsTests`) — `CreateOrReplaceArgumentsAssignsInstanceNodeIds`, `CreateOrReplaceArgumentsKeepsCallerAssignedNodeId`, `CreateOrReplaceArgumentsHonoursTheAssignmentOptOut`, `CreateOrReplaceEnumStringsAssignsInstanceNodeIds`. Pump DI server regression (`PumpInstanceNodeIdRegressionTests`, added on review feedback): \| Test \| Asserts \| \| --- \| --- \| \| `ConfiguredPumpExposesTheFullModelSurface` \| The exact set of 77 model identifiers a configured pump materialises. \| \| `EveryPumpInstanceExposesTheSameGeneratedModelSurface` \| A second pump exposes the identical generated surface, minus what the fluent `Configure` pass adds. \| \| `MaterialisedPumpNodeIdsComeFromTheNodeIdFactory` \| Every NodeId is `{parentIdentifier}_{symbolicName}` in the parent's namespace — minted by `PumpNodeManager.New`. \| \| `MaterialisedPumpNodeIdsAreUniqueAcrossInstances` \| No NodeId is null or shared between two instances of the type. \| \| `GeneratedFactoryAssignsInstanceNodeIdsBeforeRegistration` \| The factory output already carries per-instance NodeIds before `AddPredefinedNodeAsync` applies its defensive rebase. \| \| `AlarmSubtreeKeepsStandardDeclarationNodeIds` \| Pins the alarm subtree's standard-namespace ids — a pre-existing gap outside the generated helpers. \| The 77-identifier baseline was captured by running the same subtree walk against the pre-change commit and diffing it against the post-change run: identical — same node set, same NodeIds. ### Local test runs \| Suite \| net10.0 \| net48 \| net472 \| \| --- \| --- \| --- \| --- \| \| `Opc.Ua.SourceGeneration.Core.Tests` \| 3722 passed \| 3721 passed \| — \| \| `Opc.Ua.SourceGeneration.Tests` \| 81 passed \| 81 passed \| — \| \| `Opc.Ua.SourceGeneration.Stack.Tests` \| 90 passed \| — \| — \| \| `Opc.Ua.Types.Tests` \| 8083 passed \| 8076 passed \| — \| \| `Opc.Ua.Di.Tests` \| 303 passed \| 303 passed \| 277 passed \| \| `Opc.Ua.Core.Tests` \| 4134 passed \| — \| — \| \| `Opc.Ua.Gds.Tests` \| 1095 passed \| — \| — \| \| `Opc.Ua.History.Tests` \| 506 passed \| — \| — \| \| `Opc.Ua.Server.Tests` \| 3626 passed, 1 failed \| 3574 passed, 1 failed \| — \| The single `Opc.Ua.Server.Tests` failure — `ServerFluentApiHostingTests.ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync` — is pre-existing and unrelated; it reproduces on a stashed clean `master` tree. > [!NOTE] > The source-generation test harness compiles generated model code against a hand-written `Opc.Ua` API stub (`tests/Opc.Ua.SourceGeneration.Core.Tests/CompilerUtils.cs`, shared by three test projects). Its 73 `CreateOrReplace<Child>` declarations were updated to the new signature — without that, every generated call site passing `assignInstanceNodeIds: false` failed to bind. `NodesetMethodArgumentTests` pinned the exact emitted call and was updated to the new one for the same reason as the `NodeStateGeneratorTests` expectations. > [!NOTE] > `master` has been merged into this branch. The only conflict was in `NodeStateGeneratorTests.cs`, where master rewrapped the expected call-site strings while this branch appended `assignInstanceNodeIds: false` to them — kept master's wrapping and re-applied the argument. Master's two new templates (`Create_MethodArguments`, `Assign_MethodArgumentValues`) call `CreateOrReplace{Input,Output}Arguments` while building a declaration subtree, so they now pass `assignInstanceNodeIds: false` to preserve their stated intent of leaving the property node's identity untouched. > [!TIP] > Changing a source generator leaves dependent projects' cached generated output stale. If a local build reports `CS1739: ... does not have a parameter named 'assignInstanceNodeIds'` or `CS0109` on a generated `CreateOrReplace*`, run `dotnet build <project> -t:Rebuild`. Clean CI builds are unaffected. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b019f3a2-f58d-4f7e-93db-c8a0dec656e2	2026-07-28 21:22:14 +02:00
Marc Schier	275c3800b5	[Server] Add live NodeManager lifecycle support (#4094 ) # Description Adds supported live add, reload, and remove lifecycle operations for NodeManager instances on a running server. The change introduces a DI-backed `INodeManagerLifecycle` service with direct `StandardServer` access, generation-aware registration handles, copy-on-write routing, transactional prepare/commit/rollback, runtime NodeSet convenience APIs, append-only namespace updates, model and semantic change notifications, additive-compatible complex type priming, and complete teardown behavior. Lifecycle publication is coordinated with requests, sessions, subscriptions, monitored items, continuation points, and shutdown so retired managers cannot remain reachable or be disposed while still in use. Compatible monitored items transfer to replacement generations without a transient bad status. Removed or incompatible Nodes publish `BadNodeIdUnknown` as required by OPC UA Part 4 section 5.8.4.1, retain their MonitoredItem identity, and automatically recover when a compatible Node with the same NodeId returns. Registration is documented end to end in the new [docs/NodeManagerRegistration.md](docs/NodeManagerRegistration.md), which covers compile-time, startup, and runtime registration together with the guarantees for MonitoredItems, Browse continuation points, namespace indexes, DataTypes, and change notifications. ## Review feedback addressed Interface surface reduced, as requested: - `INodeManagerMonitoredItemTracker` folded into `ISubscriptionMonitoredItemLifecycle`. - `INodeManagerMutationCoordinator` and the asynchronous MonitoredItem recovery entry point folded into `IDynamicNodeManagerHost`. - `ISessionClosingRegistry` folded into `IServerInternal`, then replaced by `ISession.IsClosing`. - `ISubscriptionDeletionRegistry` folded into `ISubscriptionManager`, then removed entirely. - `INodeManagerContinuationPointTracker` folded into `ISession`. - The synchronous recovery entry point kept on its own `ISyncNodeManagerMonitoredItemRecovery`, because `AddPredefinedNode` cannot await; it still routes through the asynchronous `INodeManagerMonitoredItemLifecycle` that `AsyncNodeManagerAdapter` exposes. - `IMonitoredItemLifecycle` renamed to `IDetachableMonitoredItem`. Reach into the Subscription area reduced to the minimum the feature needs: - The lifecycle feature itself leaves `SubscriptionManager` almost untouched: two guards that reject a request from a Session that is closing, on `CreateSubscription` and on `TransferSubscriptions`, because it would create state that is about to be torn down. The deletion claim registry, the session closing gates, the closed-session bookkeeping and the deletion restore paths introduced by earlier revisions are gone. Transferring a subscription away from a session that is closing stays allowed, since that is the recovery path the service exists for; the race that exposed is fixed by serialising source-queue closure with the transfer rather than by rejecting it. - Queueing is disabled again for a MonitoredItem with `QueueSize <= 1`, as before this PR. Without a queue the Client is served the last sampled value, so the missing-Node notification simply becomes that value; losing values is expected for a MonitoredItem without queuing. - What remains of the notification is one `DataValue` plus one bool in `DataChangeQueueHandler`, mirroring the existing overflow pair, which stops it being the value discarded when a configured queue is full. - `MonitoredItem` keeps its original publish path, its original queue creation and restore paths, its original triggering behaviour, and no per-epoch bookkeeping. - `IStoredMonitoredItem` carries `IsDeleted` and `IsDetached`, so a restored item reports the state it was stored with rather than one inferred from the last value's StatusCode. Other feedback: - Documented `HostedNodeManagerLifecycle`, `IDynamicNodeManagerHost`, `NodeManagerRoutingTable`, and the `RequestManager` validation and request scopes. - Renamed `ValidateMonitoredItemAsync` to `CanAttachMonitoredItemAsync` and `RestoreMonitoredItemAsync` to `RecoverMonitoredItemAsync`, so the lifecycle API no longer collides with the durable Subscription `RestoreMonitoredItem` path used at server startup. - Removed the `CustomNodeManager2` dependency from the synchronous MonitoredItem recovery path. - `DataChangeQueueHandler` is back to the two constructors 1.5.378 shipped, each taking an optional `TimeProvider`. - Session closing is a one-way latch, and the Session is removed even when teardown throws. - Documented what a custom NodeManager has to implement to take part in live lifecycle operations, and which NodeManager implements `INodeManagerReloadParticipant` today. ## Fixes found during review - The guard that stops a NodeManager lifecycle operation being started from inside a service handler never fired. It was backed by an `AsyncLocal` written inside `async ValidateRequestAsync`, which is invisible to the caller that awaited it, so a lifecycle call from a NodeManager callback waited for its own request and held the lifecycle semaphore forever. The mark is now applied by the service dispatcher, and the drain is bounded so no path can wedge the lifecycle permanently. - `ValidateRequestAsync` is no longer virtual. An override that rejected a request after calling `base` left it registered forever; subclasses now use `OnRequestValidatedAsync`, whose failures complete the request. - A MonitoredItem deleted and disposed while a reload was handing it to the replacement NodeManager could be attached anyway. The item now reserves itself for the hand-over and the transition takes it back if it was disposed meanwhile. - The Overflow bit is recorded and applied when the value is published, as the surrounding design does, rather than written into the value at enqueue time. - MonitoredItem cleanup for a Subscription being deleted no longer passes null entries to `DeleteMonitoredItemsAsync`. - Browse continuation points of a retired NodeManager are persisted and disposed outside the Session lock. ## Open review discussions Three threads are open, replied to and awaiting reviewer decisions: separating service-call dispatch from NodeManager handling in `MasterNodeManager` (tracked as #4098), the request-scoping shape in `StandardServer`, and how much of the async MonitoredItem service implementation should stay in `Subscription`. Issue #4102 records a specification ambiguity found while implementing the missing-Node notification: Part 4 §5.8.4.1 makes it mandatory, but the protected over-capacity queue entry §5.13.1.5 defines applies to `EventQueueOverflowEventType` only, so the specification does not say how it survives a full queue. ## Validation - Full `Opc.Ua.Server.Tests` on net10.0: 3,768 passed, 5 skipped. - Lifecycle, MonitoredItem, Subscription and continuation point suites: 185 passed on net10.0 and 75 on net48. - Server builds complete with zero warnings or errors. - Two failures are pre-existing and unrelated: `DurableDataValueQueueVerifyReferenceBatchingAsync` (flaky `BatchPersistor` file contention, passes in isolation on net10.0 and net48) and `ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync` (fails on a clean tree with these changes stashed). ## Related Issues - Fixes #3993 - Follow-up tracked in #4098	2026-07-28 16:06:28 +02:00
Marc SchierandCopilot	baffd5f7e3	Fix signed preview pack build (CS8002) (#4105 ) # Description Fixes the failing OPCFoundation.UA-.NETStandard Nuget Preview pipeline ([build 16022](https://opcfoundation.visualstudio.com/opcua-netstandard/_build/results?buildId=16022&view=results)), where both the Pack Nugets Debug and Pack Nugets Release jobs failed. Root cause The preview pipeline builds `preview-pack.slnx` with `/p:SignAssembly=true` (see `.azurepipelines/preview.yml`). Strong-naming promotes `CS8002` — "Referenced assembly ... does not have a strong name" — to a build-breaking error for every reference to an unsigned assembly. `common.props` only suppresses `CS8002` for net5.0-compatible target frameworks, where strong-name identity is no longer enforced. The `Dekaf` package (the default, AOT-compatible Kafka backend) is not strong-named (`PublicKeyToken=null`) and ships no signed variant, so `Opc.Ua.PubSub.Kafka` failed to compile on the remaining TFMs: ``` CSC : error CS8002: Referenced assembly 'Dekaf, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' does not have a strong name. [src\Opc.Ua.PubSub.Kafka\Opc.Ua.PubSub.Kafka.csproj::TargetFramework=net472] [src\Opc.Ua.PubSub.Kafka\Opc.Ua.PubSub.Kafka.csproj::TargetFramework=net48] [src\Opc.Ua.PubSub.Kafka\Opc.Ua.PubSub.Kafka.csproj::TargetFramework=netstandard2.1] ``` These three errors were the only errors in the build. Fix Suppress `CS8002` in `Opc.Ua.PubSub.Kafka.csproj`, scoped precisely to the case that needs it: ```xml <PropertyGroup Condition="'$(SignAssembly)' == 'true' AND !$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net5.0'))"> <NoWarn>$(NoWarn);CS8002</NoWarn> </PropertyGroup> ``` - `CS8002` is only ever emitted when the output assembly is itself strong-named, so outside the signed pack build the suppression is inert rather than merely redundant. - The `net5.0` compatibility check narrows it to `net472;net48;netstandard2.1`. It reuses the same `IsTargetFrameworkCompatible` predicate `common.props` uses, rather than hardcoding a TFM list, so the two stay in sync if `LibxTargetFrameworks` changes. - If `Dekaf` ever ships a strong-named build, the suppression becomes dead rather than silently masking a real reference problem. Referencing a non-strong-named assembly is otherwise benign — it only surfaces in the signed pack build. Upgrading is not an option, as no signed `Dekaf` build exists. The same class of suppression already exists for the unsigned `Makaretu.Dns` packages in `Opc.Ua.Lds.Server.csproj` and for `SourceGenerator.Foundations.Contracts` in the source-generator projects. Verification - Reproduced locally with a throwaway `.snk` and confirmed `CS8002` fires without the fix. - Ran the exact CI sequence with the fix — `.azurepipelines/generate-slnx.ps1`, then `dotnet build preview-pack.slnx --configuration Release /p:SignAssembly=true /p:AssemblyOriginatorKeyFile=...` across all 56 packable projects and every TFM (`net472;net48;netstandard2.1;net8.0;net9.0;net10.0`): Build succeeded, 0 errors. - `Opc.Ua.PubSub.Kafka` builds with 0 warnings, 0 errors on all six TFMs both with and without `SignAssembly=true`. - The only downstream consumer of the project, `ConsoleReferencePubSubClient`, targets `net10.0` only, where `CS8002` is already suppressed globally — so it is unaffected. ## Related Issues No tracking issue — this is a CI-only regression in the nightly preview pack build, reported directly by the pipeline run linked above. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. — _N/A: build-configuration-only change (a scoped `NoWarn` entry); there is no runtime behavior to test. The fix is verified by reproducing the signed pack build, as described above._ - [x] I have added all necessary documentation. — _Rationale is captured as an inline comment next to the suppression, matching the style used for the other `CS8002` suppressions in the repo._ - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. — _The full signed pack build reports 0 errors; the 8 remaining warnings are pre-existing `CA2000` occurrences in `samples/ConsoleReferenceClient` that are unrelated to this change._ - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. — _No compiled code changes; `Opc.Ua.PubSub.Kafka` builds clean on all six TFMs, signed and unsigned._ - [ ] 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: fe9d9cfd-861f-4306-9505-eb134d554c00	2026-07-28 15:15:34 +02:00
Marc Schier	fde50c5700	Enforce ActivateSession transfer and reconnect security (OPC 10000-4 §5.7.3.1) (#4050 ) # Description This draft isolates the Session/ActivateSession, reconnect, and distributed Session security work split from #4021. ### Failure - `ActivateSession` could transfer a Session to a new SecureChannel without consistently enforcing the original client certificate, MessageSecurityMode, SecurityPolicy, and exact OPC `ClientUserId` semantics. - Missing or legacy distributed Session security state did not provide enough information to validate transfer safely. - Malformed or reused nonces, concurrent activation, and cancellation could leave activation state inconsistent. - Client reconnect could treat recoverable Session-loss/security responses as fatal, reuse a Session after client-certificate rotation, or report a managed channel ready before Session recreation completed. ### Fix - Serialize activation, consume server nonces once, validate application signatures and user tokens asynchronously, and enforce transfer continuity against the original channel and authenticated identity. - Persist versioned transfer-security state for distributed Sessions and fail closed when certificate, identity, activation, or nonce state is absent or invalid. - Validate server signatures/nonces on the client, recreate Sessions for certificate rotation and recoverable activation failures, and await participant recreation before transitioning the managed channel to `Ready`. - Make the restart recovery integration test use bounded reconnect cycles instead of a fixed startup delay. ### Specification - OPC 10000-4 §5.7.2.2 — CreateSession signatures and nonces. - OPC 10000-4 §5.7.3.1 — ActivateSession identity continuity and SecureChannel transfer. - OPC 10000-4 §6.1.8 — application nonce generation, validation, and reuse requirements. ### Validation - `git diff --check` - Release `net10.0`: `Opc.Ua.Redundancy.Server.Tests` 567/567 plus the targeted server Session, Subscription transfer, ClientUserId, client reconnect and certificate rotation suites pass. - Release `net48`: the same redundancy suite (567/567) and the targeted Session/Subscription suites pass. - Full `Opc.Ua.Server.Tests` run on `net10.0`: 3633/3634. The single failure, `ServerFluentApiHostingTests.ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, reproduces identically on the unmodified commit and is a local certificate-store path issue rather than a regression. These were targeted validation runs; no long-soak result is claimed.	2026-07-28 12:07:02 +02:00
Marc Schier	5b9bcedc62	Add OPC UA ISA-95 Common Model and Job Control support (#4089 ) # Description Adds first-class support for the OPC UA ISA-95 companion specifications: - OPC-10030 Common Model. - OPC-10031-4 Job Control V1. - OPC-10031-4 Job Control V2. The change introduces `Opc.Ua.ISA95`, `Opc.Ua.ISA95.Client`, and `Opc.Ua.ISA95.Server`, together with a NativeAOT-compatible minimal server sample and comprehensive tests. ## Highlights - Compiles the three canonical NodeSets and identifier mapping CSV files through the source generator. - Adds NodeSet identifier mapping validation, typed method arguments/results, companion event records, and generated method argument nodes. - Provides typed Common Model builders with normative relationships and an injectable `GeoSpatialLocationType` provider. - Implements lossless V1/V2 Job Control providers over a shared version-neutral store, including state paths, status events, comments, response queries, catalog changes, and cohesive DI registration. - Adds subtype-aware typed clients, standalone encodeable registration, `IIsa95ClientFactory`, and V2 status-event streaming. - Adds the `MinimalIsa95Server` sample, package documentation, and an ISA-95 guide with conformance and HA guidance. - Tracks typed RSL/GPOS integration as follow-up issue #4095. ## Validation - `Opc.Ua.ISA95.Tests`: 137/137 passed on `net10.0` and `net48`. - Source-generator suites: 81/81 model, 90/90 stack, 3718 passed / 8 skipped core. - `EventRecordFieldReaders` feedback tests: 6/6 on `net10.0` and `net48`. - `EventRecordFieldReaders` changed-method patch coverage: 100% line coverage (36/36) and 100% branch coverage (22/22). - `Opc.Ua.ISA95`, `.Client`, and `.Server` build across all configured target frameworks without new diagnostics. - Cobertura line coverage: 88.08% model, 90.04% client, 91.19% server. - NativeAOT sample publishes and passes a live typed V2 Store/status-event validation. - The ISA-95 NativeAOT smoke test passes in the source-generated native test executable. - OPC UA KB NodeSet validation reports zero errors for Common, V1, and V2. The broader NativeAOT executable still reports unrelated existing GDS and complex-type test failures; the ISA-95-specific native test passes. ## Node state generator fixes Generating method arguments inline for imported nodesets regressed every model that already emits `InputArguments` / `OutputArguments` child node factories: - The inline block ran before the child factories. `MethodState.CreateOrReplaceInputArguments` keeps the property that was assigned first, so the child carrying the well-known NodeId was dropped and a NodeId-less placeholder remained in the address space. This broke browsing (reference descriptions without a target) and made method arguments unreachable at their well-known NodeIds. - The `MethodDeclarationId` fallback keyed on an autogenerated display name redirected every declared method of the standard address space to its own NodeId, which made method calls fail. The inline block is now emitted after the child factories and reduced to a value assignment when a child factory exists. The value is only re-emitted when an argument references a namespace other than OPC UA, because the value of an imported property is the verbatim XML of the source nodeset whose namespace indices do not survive the import. The `MethodDeclarationId` fallback is restricted to declarations without a resolvable NodeId. The generated OPC UA address space is byte-identical to `master` again. ## Related Issues - Fixes #4088 - Follow-up: #4095	2026-07-28 12:01:57 +02:00
Marc Schier	38c7356bc7	Add generic xRegistry libraries (abstract registry base model) (#4097 ) # Description xRegistry is the abstract registry base model: a registry-agnostic, content-addressed resource registry. It knows nothing about what a resource contains — a concrete registry supplies its own companion namespace and a fingerprinting strategy and reuses everything else. The PubSub Schema Registry in #4007 is the first specialization (its resources are schema documents), and #4093 (WoT Binding) carries a byte-identical copy of the same base NodeSet, so a shared canonical home is worth having. ## Packages \| Package \| Depends on \| Contains \| \| --- \| --- \| --- \| \| `OPCFoundation.NetStandard.Opc.Ua.XRegistry` \| `Opc.Ua.Core` \| Abstract base companion model (compiled by the model source generator), `XRegistryWellKnown`, `IResourceContentIdProvider` \| \| `OPCFoundation.NetStandard.Opc.Ua.XRegistry.Client` \| + `Opc.Ua.Client` \| `XRegistryClient` (abstract) / `GenericXRegistryClient` (sealed), built on the generated ObjectType proxies \| \| `OPCFoundation.NetStandard.Opc.Ua.XRegistry.Server` \| + `Opc.Ua.Server` \| Fast-path / registration / federation node managers, `XRegistryServerOptions`, `IXRegistryResourceStore` \| `Opc.Ua.XRegistry` deliberately depends on neither SDK, so a codec or a shared contracts assembly can reference the identity abstraction without pulling in the client or the server. ## The model is compiled, not loaded The companion model is compiled into the assemblies by the OPC UA model source generator. `Opc.Ua.XRegistry.NodeSet2.xml` is an `AdditionalFiles` generator input only — there is no `EmbeddedResource`, no runtime NodeSet parsing and no hand-written NodeId tables for anything the model declares. All three node managers load it the same way: ```csharp protected override NodeStateCollection LoadPredefinedNodes(ISystemContext context) { return new NodeStateCollection().AddOpcUaXRegistry(context); } ``` `XRegistryWellKnown` is reduced to the namespace URI plus the identifiers of the instances a registry materializes at runtime, which live above the model's own 63000-63999 range so they can never collide with it. ## What it does - Content-derived identity — `IResourceContentIdProvider` maps a document + format to a fingerprint and names the (canonicalization, hash) algorithm. Identity is derived from the bytes, not assigned by the server, so it is stable across registries — which is what makes de-duplication and federation work. - Opaque-NodeId fast path — a registered resource is reachable at an Opaque `NodeId` whose Identifier is the raw content-id bytes, so a consumer that received the id on the wire resolves the document in a single `Read`: no Browse, no fingerprint recomputation. The client reads it through the `ReadBytesAsync` session extension, so a document larger than `MaxByteStringLength` is fetched with range-based reads rather than failing. Optionally pre-publishes a seed resource so a fresh server can resolve one resource before any registration. - The model's own lifecycle — the registry root is a `RegistryType` instance serving `CreateGroup` / `GetOrCreateGroup`; each group serves `CreateResource` / `GetOrCreateResource`; and because `ResourceType : FileType`, the document is the file — content is streamed through the inherited `Open` / `Read` / `Write` / `Close`. `Close` fingerprints the accumulated bytes and publishes the Opaque fast-path node at runtime. Identical bytes reuse the existing node, whose lifetime is reference counted so it survives until the last resource resolving to those bytes is gone and is retired when a resource is re-written with different content. - Optimistic concurrency — `Delete(ExpectedEpoch)` on both resources and groups, and the same epoch check on label mutations, so a caller working from a stale read is rejected rather than clobbering a concurrent change. - Labels — `RegistryType`, `GroupType` and `ResourceType` each expose an `AttributesType` Object with `AddAttribute` / `RemoveAttribute`; labels are published as addressable `String` Properties. - Federation — a resource hosted by another registry is published as a real `ResourceType` instance whose `ExternalReference` (`ExpandedNodeId` + `ServerIndex` into the local `ServerArray`) and `ResourceUrl` carry the link and whose `Xid` carries the content-id. Because it is an ordinary `ResourceType`, a generic client drives a federated resource through exactly the same proxy as a local one. ## Extensibility A domain registry subtypes the base model — the PubSub Schema Registry declares `SchemaFileType : ResourceType` — and the generator mirrors that hierarchy across assemblies, so the proxy chain follows automatically (`SchemaFileTypeClient : ResourceTypeClient : FileTypeClient`). ```text abstract XRegistryClient ├── sealed GenericXRegistryClient // any registry namespace ├── SchemaRegistryClient (domain) └── WotRegistryClient (domain) ``` * A domain client derives from `XRegistryClient` and inherits the whole lifecycle. Helpers written as extension methods over a base proxy (such as `WriteDocumentAsync` on `ResourceTypeClient`) are callable on the domain proxy with no inheritance in the client layer. * A generic client still drives a domain registry, because a domain instance is an instance of the base type. This is covered by a test that defines a domain subclass over the base. ## State Document bytes live behind an injectable `IXRegistryResourceStore`. Because a resource is a `ResourceType`, which is a `FileType`, the store mirrors the file access model: reads and writes are offset and length based, so a document never has to be materialized as a whole and a ranged read touches only the bytes it needs. The document is transferred with the inherited `FileType` Methods, whose mode bits are honoured: a write that does not set `EraseExisting` starts from the stored bytes and replaces only the range it writes rather than truncating the rest. A handle is valid only on the resource and the session that opened it, and a session's handles are released when it closes. Two implementations ship: `InMemoryResourceStore` (the default, in-process) and `FileSystemResourceStore`, built on the `IFileSystem` abstraction so documents outlive the process and a shared volume can back a cluster. `XRegistryResourceStoreContractTests` is a reusable fixture that validates any implementation against the contract. ## Transport security Registry writes always require a `SignAndEncrypt` secure channel — a document and its content-derived identity are integrity-critical, so every mutation is rejected with `BadSecurityModeInsufficient` on a channel that is only signed or unprotected. This is not configurable. Reads are allowed on any secure channel by default; `RequireEncryptionForReads` extends the requirement to them when the documents themselves are confidential. An in-process call carries no channel and is always allowed. ## Hardening The lifecycle Methods are remotely callable, so every unbounded dimension is bounded via `XRegistryServerOptions`; exceeding a bound fails the call rather than the server: \| Option \| Default \| Enforced on \| Status code \| \| --- \| --- \| --- \| --- \| \| `MaxConcurrentUploads` \| 64 \| `CreateResource`, `Open` \| `BadTooManyOperations` \| \| `MaxResourceBytes` \| 16 MiB \| `Write` \| `BadRequestTooLarge` \| \| `MaxRegisteredResources` \| 4096 \| `CreateResource` \| `BadTooManyOperations` \| ## A source generator bug fixed along the way Building on the compiled model surfaced a defect in shared generator infrastructure. `NodeSetToModelDesign.CreateDecoder` decoded values with an empty `EncodeableFactory`, so a NodeSet2's method `InputArguments` / `OutputArguments` — encoded as a list of `Argument` ExtensionObjects — never decoded, while `HasArguments` was still set. Every NodeSet2-sourced model in the stack was silently generating argument-less `MethodState`s and ObjectType proxies. ModelDesign-sourced models (the core stack) were unaffected, which is why this went unnoticed. Fixed in `b87726af` with a regression test that fails without it. Two defects in the xRegistry NodeSet itself were fixed at the same time: `BrowseName="1:InputArguments"` → ns-0 `InputArguments` (published NodeSets never namespace-qualify these), and the `CreateResource` / `GetOrCreateResource` output `VersionId` renamed to `AssignedVersionId`, which collided with the input of the same name and produced a duplicate C# parameter. ## Other fixes applied while extracting - `XRegistryRegistrationNodeManager` uses `System.Threading.Lock` instead of an `object`-typed sync root, per the repository locking rules. - Dropped the `InternalsVisibleTo` entries granting internals to `Opc.Ua.PubSub.Server.Tests`. That project only consumes public `XRegistryWellKnown` API. - The `UA.slnx` edit in #4007 accidentally replaced the `tests/Opc.Ua.Redundancy.Kubernetes.Tests` entry instead of adding a line, silently dropping that project from the solution. The entry is preserved here and the change is purely additive. ## Tests `tests/Opc.Ua.XRegistry.Tests` (168) covers the group and resource lifecycles, the `FileType` transfer, delete and epoch conflicts, labels, federation, both resource stores, the fast path, transport security and the extensibility contract — positive and negative paths. Passing on `net10.0` and `net48`. Patch line coverage is 98.5%. Also adds `tests/Directory.Build.props` / `.targets`: the .NET 10 SDK prunes `Microsoft.Extensions.Logging.Abstractions` out of the test output, and coverlet's Mono.Cecil resolver only searches the output directory, so it silently fails to instrument every module that references `ILogger` and reports 0% coverage for them. These force the package back into the restore graph and copy its implementation next to the test binary. ## Documentation New [`docs/XRegistry.md`](docs/XRegistry.md), linked from the documentation index.	2026-07-28 08:45:03 +02:00
dependabot[bot]	7e69b4ced5	Bump actions/setup-node from 6 to 7 (#4101 ) Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6 to 7. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/actions/setup-node/releases">actions/setup-node's releases</a>.</em></p> <blockquote> <h2>v7.0.0</h2> <h2>What's Changed</h2> <h3>Enhancements:</h3> <ul> <li>Add cache-primary-key and cache-matched-key as outputs by <a href="https://github.com/gowridurgad"><code>@gowridurgad</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1577">actions/setup-node#1577</a></li> <li>Migrate to ESM and upgrade dependencies by <a href="https://github.com/gowridurgad"><code>@gowridurgad</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1574">actions/setup-node#1574</a></li> </ul> <h3>Bug fixes:</h3> <ul> <li>Remove dummy NODE_AUTH_TOKEN export by <a href="https://github.com/gowridurgad"><code>@gowridurgad</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1558">actions/setup-node#1558</a></li> <li>Only use <code>mirrorToken</code> in <code>getManifest</code> if it's provided by <a href="https://github.com/deiga"><code>@deiga</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1548">actions/setup-node#1548</a></li> </ul> <h3>Documentation updates:</h3> <ul> <li>Add documentation for publishing to npm with Trusted Publisher (OIDC) by <a href="https://github.com/chiranjib-swain"><code>@chiranjib-swain</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1536">actions/setup-node#1536</a></li> <li>docs: Update restore-only cache documentation by <a href="https://github.com/priya-kinthali"><code>@priya-kinthali</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1550">actions/setup-node#1550</a></li> <li>docs: Update caching recommendations to mitigate cache poisoning risks by <a href="https://github.com/chiranjib-swain"><code>@chiranjib-swain</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1567">actions/setup-node#1567</a></li> </ul> <h3>Dependency update:</h3> <ul> <li>Upgrade <code>@actions/cache</code> to 5.1.0, log cache write denied by <a href="https://github.com/jasongin"><code>@jasongin</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1569">actions/setup-node#1569</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/chiranjib-swain"><code>@chiranjib-swain</code></a> made their first contribution in <a href="https://redirect.github.com/actions/setup-node/pull/1536">actions/setup-node#1536</a></li> <li><a href="https://github.com/deiga"><code>@deiga</code></a> made their first contribution in <a href="https://redirect.github.com/actions/setup-node/pull/1548">actions/setup-node#1548</a></li> <li><a href="https://github.com/jasongin"><code>@jasongin</code></a> made their first contribution in <a href="https://redirect.github.com/actions/setup-node/pull/1569">actions/setup-node#1569</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/setup-node/compare/v6...v7.0.0">https://github.com/actions/setup-node/compare/v6...v7.0.0</a></p> <h2>v6.5.0</h2> <h2>What's Changed</h2> <ul> <li>Update <code>@actions/cache</code> to 5.1.0 and add security overrides for undici and fast-xml-parser by <a href="https://github.com/HarithaVattikuti"><code>@HarithaVattikuti</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1579">actions/setup-node#1579</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/setup-node/compare/v6.4.0...v6.5.0">https://github.com/actions/setup-node/compare/v6.4.0...v6.5.0</a></p> <h2>v6.4.0</h2> <h2>What's Changed</h2> <h3>Dependency updates:</h3> <ul> <li>Upgrade <a href="https://github.com/actions"><code>@actions</code></a> dependencies by <a href="https://github.com/Copilot"><code>@Copilot</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1525">actions/setup-node#1525</a></li> <li>Update Node.js versions in versions.yml and bump package to v6.4.0 by <a href="https://github.com/priya-kinthali"><code>@priya-kinthali</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1533">actions/setup-node#1533</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/Copilot"><code>@Copilot</code></a> made their first contribution in <a href="https://redirect.github.com/actions/setup-node/pull/1525">actions/setup-node#1525</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/setup-node/compare/v6...v6.4.0">https://github.com/actions/setup-node/compare/v6...v6.4.0</a></p> <h2>v6.3.0</h2> <h2>What's Changed</h2> <h3>Enhancements:</h3> <ul> <li>Support parsing <code>devEngines</code> field by <a href="https://github.com/susnux"><code>@susnux</code></a> in <a href="https://redirect.github.com/actions/setup-node/pull/1283">actions/setup-node#1283</a></li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/setup-node/commit/820762786026740c76f36085b0efc47a31fe5020"><code>8207627</code></a> Migrate to ESM and upgrade dependencies (<a href="https://redirect.github.com/actions/setup-node/issues/1574">#1574</a>)</li> <li><a href="https://github.com/actions/setup-node/commit/04be95cf3511ea51ebf9f224ddfb99cc7ab87cd4"><code>04be95c</code></a> Add cache-primary-key and cache-matched-key as outputs (<a href="https://redirect.github.com/actions/setup-node/issues/1577">#1577</a>)</li> <li><a href="https://github.com/actions/setup-node/commit/7c2c68d20d402ed6a201ada70a81341941093140"><code>7c2c68d</code></a> docs: Update caching recommendations to mitigate cache poisoning risks (<a href="https://redirect.github.com/actions/setup-node/issues/1567">#1567</a>)</li> <li><a href="https://github.com/actions/setup-node/commit/6a61c0375d66246de94630495909f12cf8dac84d"><code>6a61c03</code></a> Merge pull request <a href="https://redirect.github.com/actions/setup-node/issues/1569">#1569</a> from jasongin/update-actions-cache-5.1.0</li> <li><a href="https://github.com/actions/setup-node/commit/30eb73b41ded577900c1ebf968ef95cdf8f7434f"><code>30eb73b</code></a> Resolve high-severity audit issues</li> <li><a href="https://github.com/actions/setup-node/commit/4e1a87a501d0302f99e30e2748568adcb388d09f"><code>4e1a87a</code></a> Update dist</li> <li><a href="https://github.com/actions/setup-node/commit/360237f0c01778d0c17291f75c56d6feae4f7574"><code>360237f</code></a> Strict equality</li> <li><a href="https://github.com/actions/setup-node/commit/4f8aac5beb2f0854bc79651567a18c67eb0b9de3"><code>4f8aac5</code></a> Bump <code>@actions/cache</code> to 5.1.0, log cache write denied</li> <li><a href="https://github.com/actions/setup-node/commit/f4a67bbeca970f103397d3d2b9462cf787cd2980"><code>f4a67bb</code></a> Only use <code>mirrorToken</code> in <code>getManifest</code> if it's provided (<a href="https://redirect.github.com/actions/setup-node/issues/1548">#1548</a>)</li> <li><a href="https://github.com/actions/setup-node/commit/0355742c943ddb13ca8a6b700f824231caa91e75"><code>0355742</code></a> Remove dummy NODE_AUTH_TOKEN export (<a href="https://redirect.github.com/actions/setup-node/issues/1558">#1558</a>)</li> <li>Additional commits viewable in <a href="https://github.com/actions/setup-node/compare/v6...v7">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=actions/setup-node&package-manager=github_actions&previous-version=6&new-version=7)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-07-27 14:26:45 +02:00
Copilot	a03bca97cb	Remove TraceConfiguration and related APIs (#4092 ) # Description This change removes the remaining `TraceConfiguration`-driven apply/setter API path and aligns configuration startup with the 2.0 telemetry model (`ITelemetryContext`/`ILoggerFactory`). It also updates migration docs to make the removal explicit and actionable. - Core API removals - Deleted `TraceConfiguration.ApplySettings()` implementation file. - Removed runtime apply calls from configuration/server startup paths. - Removed fluent builder trace setters: - `SetOutputFilePath(...)` - `SetDeleteOnLoad(...)` - `SetTraceMasks(...)` - Simplified builder interface inheritance accordingly. - Call-site and test fixture updates - Removed usage of deleted trace fluent APIs in affected test infrastructure and fixtures. - Updated GDS log readers to tolerate missing `TraceConfiguration` output path safely. - Migration guidance - Updated migration docs to explicitly state removal of `TraceConfiguration` apply APIs and direct users to telemetry-based logging configuration. ```csharp // Removed API pattern app.Build(uri, product) .AsClient() .SetOutputFilePath("trace.log") .SetTraceMasks(Utils.TraceMasks.Error); // New direction // Configure logging via ITelemetryContext / ILoggerFactory at composition/startup. ``` ## Related Issues - OPCFoundation/UA-.NETStandard#4091 ## 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. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #4091 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>	2026-07-26 00:50:59 +02:00
Copilot Marc Schier Copilot Marc Schier	16dd14f420	Improve MCP server coverage and agent usability (#4080 ) ## Summary - Expand deterministic MCP coverage across host startup, sessions, configuration, NodeSet export, PKI, packet diagnostics, and PubSub. - Fix Streamable HTTP startup by supplying the OPC UA client application identity. - Add client-visible required-argument errors and normalize generated tool schemas with explicit `required` arrays. - Introduce bounded `core`, `services`, `administration`, `pubsub`, and `diagnostics` tool profiles while retaining `full` as the current-major default. - Split configuration updates into focused transport, client, and security tools while retaining `SetConfiguration` in the `full` compatibility profile. - Improve tool descriptions/examples and add pinned, deterministic `mcpgrade` static/probe CI. ## Compatibility and behavior - Existing stdio clients continue to receive the `full` catalog by default. - Streamable HTTP is exposed at `/mcp`; the previous root route is intentionally removed. - `--transport http` is preferred; `--transport sse` remains a deprecated current-major alias. - Diagnostics decode/replay/key tools remain behind the existing explicit security gate. - N002 is disabled only for documented OPC UA standard and compatibility-sensitive name pairs. ## Validation - Release analyzer build: 0 warnings and 0 errors. - MCP tests: 276/276 passed with focused Coverlet collection. - MCP coverage: 82.73% lines, 78.96% branches. - MCP patch coverage: 94.79% lines (291/307 changed coverable lines). - `core`: 15 tools, mcpgrade A/100, zero findings. - `full`: 78 tools, mcpgrade A/100; only documented T001, T004, and S007 findings remain. - Full stdio probe: zero C003/C004 findings. Fixes #4079 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Copilot-Session: bb615463-338e-4bbb-9779-5bf6780b8b47	2026-07-24 20:23:04 +02:00
Marc SchierandCopilot	1b46fd5887	Enforce KeyCredential authorization and ownership (OPC 10000-12 §§8.5.5-8.5.7) (#4054 ) # Description This PR splits the GDS KeyCredential changes from #4021. ## Specification - [OPC 10000-12 v1.05.07 §8.5.5](https://reference.opcfoundation.org/specs/OPC-10000-12/v1.05.07/8.5.5) requires `StartRequest` to use an encrypted SecureChannel and a Client with the `KeyCredentialAdmin` Role, `ApplicationAdmin` Privilege, or `ApplicationSelfAdmin` Privilege. The `ApplicationUri` must uniquely identify an application known to the GDS. - [OPC 10000-12 v1.05.07 §8.5.6](https://reference.opcfoundation.org/specs/OPC-10000-12/v1.05.07/8.5.6) applies the same authorization requirements, requires `FinishRequest` to use the same SecureChannel Certificate as `StartRequest`, and defines `Bad_RequestNotComplete` for a pending request. - [OPC 10000-12 v1.05.07 §8.5.7](https://reference.opcfoundation.org/specs/OPC-10000-12/v1.05.07/8.5.7) applies the same authorization requirements to `Revoke` and defines `Bad_InvalidArgument` for an unknown credential. ## Failure The KeyCredential methods previously verified only an authenticated encrypted SecureChannel. They did not enforce the specification's administrative Role or application-scoped Privileges, persist immutable application ownership, or bind `FinishRequest` to the initiating Client certificate. A pending request returned `BadNothingToDo`, invalid ownership lookups used non-conformant status codes, and failure audit text could expose exception details. ## Fix - Publish and enforce `KeyCredentialAdmin`, `ApplicationAdmin`, and `ApplicationSelfAdmin` permissions, including application scoping. - Resolve `ApplicationUri` to exactly one registered application and persist its immutable `ApplicationId`. - Bind requests to the initiating SecureChannel certificate with a SHA-256 fingerprint and compare it in fixed time when finishing. - Atomically re-check request and credential ownership, and fail closed for stores that cannot enforce the new ownership contract. - Return `BadRequestNotComplete` for pending requests and conformant invalid-argument results for unknown request or credential identifiers. - Emit success and failure audit events while redacting KeyCredential exception details. ## Validation - Full `Opc.Ua.Gds.Tests`: net10.0 (1053 passed, 42 skipped), net48 (858 passed, 237 skipped). - Review follow-up `KeyCredentialRequestStoreTests`: net10.0 (16 passed), net48 (16 passed). - `git diff --check` passed. ## Related Issues - Split from #4021. ## Checklist _Only verified claims are checked._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [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>	2026-07-24 20:02:16 +02:00
Marc Schier	e5cb64cb62	Make fluent sample security settings explicit (#4087 ) # Description Makes security-critical configuration explicit in the modern fluent client and server samples instead of relying on hidden defaults. - Adds an application-wide `ConfigureSecurity` DI callback for advanced certificate validation settings. - Makes PKI roots and certificate validation settings explicit in the Pump, Boiler, Calculator, and Minimal Client samples. - Makes the Calculator sample's endpoint policies and anonymous authentication choice explicit. - Adds consolidated developer guidance for application certificates, endpoint security, identity, resource controls, and every advanced security-builder setting. - Preserves the samples' existing automatic-trust behavior while clearly marking it as sample-only. ## Related Issues - Fixes #4085 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. Copilot-Session: 833a8855-af3c-4a85-887b-32bf38d92cfa	2026-07-24 12:56:51 +02:00
Marc SchierandCopilot	5efba99dbd	Replace legacy EventSource providers with source-generated logging (#4058 ) # Description Replace the four stack-owned `System.Diagnostics.Tracing.EventSource` providers with source-generated `ILogger` logging. - Preserve the legacy provider category names, numeric event IDs, event names, levels, message structure, and semantic structured fields. - Remove `ClientTraceFlags.EventLog` and route `ClientTraceFlags.Log` through the canonical `OPC-UA-Core` compatibility events. - Keep ChannelManager activities and metrics while moving its diagnostic events to the `Opc.Ua.ChannelManager` logger category. - Replace EventListener-based unit and stress-test collection with structured logger providers. - Document the ETW removal, retained compatibility identities, and migration path for existing consumers. ## Validation - `dotnet build UA.slnx -c Release -p:CustomTestTarget=net10.0 -p:NuGetAudit=false` - Focused Core, Client, and Server logging tests on `net10.0` and `net48` - Stress harness Release build and `SessionRecoversFromAcceptButStallAsync` - Direct AOT test application: 115 tests passed - `dotnet test UA.slnx -c Release -p:CustomTestTarget=net48 -p:NuGetAudit=false` The full `net10.0` `dotnet test UA.slnx` invocation is currently blocked by the repository's existing mixed VSTest/Microsoft.Testing.Platform configuration in `Opc.Ua.Aot.Tests`; the AOT project builds and its test executable passes directly. ## Related Issues - Related to #1262 ## 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: b0d90b8a-cde9-4fca-9a75-84f4cecb227d	2026-07-23 21:53:31 +02:00
Marc SchierandCopilot	a5dd6eae7c	Treat zero MaxNotificationsPerPublish as unlimited (OPC 10000-4) (#4047 ) # Description This focused change extracts the `MaxNotificationsPerPublish` conformance fix from #4021. It contains only `Subscription.cs`, `SubscriptionManager.cs`, and focused `SubscriptionTests`. ## Failure OPC UA defines zero as no notification limit. The message builder compared the queued count directly with zero, so an effective zero limit emitted no event or data-change notifications and left the queues pending. Separately, the revision logic treated a configured Server maximum of zero as a numeric cap, revising a non-zero Client request to zero and silently removing the Client's requested limit. ## Fix - Treat an effective zero limit as unlimited while draining event and data-change queues. - When the Client requests zero, use the Server's configured maximum, which may itself remain zero/unlimited. - When the Server maximum is zero, preserve a non-zero Client request, including `uint.MaxValue`. - When the Server maximum is finite, cap zero, `uint.MaxValue`, and larger finite Client requests to that maximum. - Cover the same revision matrix through both CreateSubscription and ModifySubscription. This implements the zero-means-unlimited semantics in OPC 10000-4 v1.05.07 Sections 5.14.2.2 and 5.14.3.2. Those Service responses do not contain a `revisedMaxNotificationsPerPublish` field; the effective value is retained by the Server-side Subscription and its diagnostics. ## Tests - `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f net10.0 --filter "FullyQualifiedName~Opc.Ua.Server.Tests.SubscriptionTests" --no-restore`: 25 passed. - `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f net48 --filter "FullyQualifiedName~Opc.Ua.Server.Tests.SubscriptionTests" --no-restore`: 25 passed. The regression tests cover zero, finite, and `uint.MaxValue` Client requests against unlimited and finite Server limits for both CreateSubscription and ModifySubscription. They also verify that zero drains both event and data-change queues. ## Related Issues - Focused split from #4021 (closed umbrella draft); no separate issue. ## 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>	2026-07-23 14:19:01 +02:00
Marc Schier	ec02804157	Do not fail Docker builds on cache export errors (#4082 ) ## Summary - keep successful Docker builds from failing when GitHub Actions cache export is unavailable - apply the non-fatal cache export setting to both Docker image workflows ## Validation - targeted rerun of the failed MCP server Docker job succeeded - changed workflow YAML parsed successfully	2026-07-23 12:23:24 +02:00
Marc SchierandCopilot	94be07cafb	Prevent stale redundant client Session swaps (OPC UA redundancy) (#4049 ) ## Failure Concurrent active-Session refreshes observed the coordinator outside the facade lock and then applied results in completion order. An older refresh could overwrite a newer Session, while a non-null to non-null swap retained the already-completed waiter and routed later async calls to the stale Session. ## Fix - Assign every refresh invocation a generation before accessing the Session and reject earlier-started refreshes after a later refresh has applied. - Reset the active-Session completion source for live Session replacements. - Cover concurrent stale refreshes and async calls immediately after a live swap. ## Standard OPC 10000-4 §6.6 redundant-client continuity requires the stable facade to route service calls through the current active Session during replica and Session transitions. ## Tests - `dotnet test tests\Opc.Ua.Redundancy.Client.Tests\Opc.Ua.Redundancy.Client.Tests.csproj -f net10.0 --no-restore --nologo --verbosity minimal` — 122 passed - `CustomTestTarget=net48 dotnet test tests\Opc.Ua.Redundancy.Client.Tests\Opc.Ua.Redundancy.Client.Tests.csproj --no-restore --nologo --verbosity minimal -m:1` — 122 passed - Review follow-up: `RedundantClientSessionTests` — 7 passed on net10.0 and 7 passed on net48. ## Split Split from #4021. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-23 10:59:02 +02:00
Marc Schier	2a4bc0fe24	Make reverse connect lifecycle fully asynchronous (#4059 ) ## Summary - add cancellation-aware asynchronous start, lazy initialization, stop, reload, and disposal for `ReverseConnectManager` - make listener replacement transactional with explicit ownership, rollback/restoration, stale watcher protection, and deterministic concurrent start/stop/dispose behavior - add injectable async configuration providers, remove the former protected configuration hooks, and retain obsolete synchronous lifecycle compatibility wrappers - integrate eager Generic Host startup plus non-host lazy startup, including custom application-configuration providers - serialize `ReverseConnectHost` lifecycle and extend raw TCP, Kestrel TCP, WSS, hosting, concurrency, rollback, and NativeAOT coverage - document the async lifecycle, hosting behavior, cancellation guarantees, provider model, and migration path ## Validation - `Opc.Ua.Client` Release build across all supported target frameworks - reverse-connect client and core tests on net10.0 and net48 - raw TCP, Kestrel TCP, and WSS integration tests on net10.0; raw TCP on net48 - NativeAOT publish and 115-test executable run Closes #4013. Related: #3985, #4009.	2026-07-23 10:40:32 +02:00
Marc Schier	3490d9cd05	Use Dekaf by default for Kafka on all target frameworks (#4038 ) # Description Upgrade the Kafka transport from Dekaf 1.2.0 to 1.7.0 and use the pure-managed Dekaf adapter by default on every supported target framework. - Enable the Dekaf adapter and its guard tests on `net472`, `net48`, `netstandard2.1`, `net8.0`, `net9.0`, and `net10.0`. - Retain the `Confluent.Kafka` adapter and factory as an opt-in DI backend through `WithConfluentKafkaClient()`. - Update the central dependency pins required by Dekaf 1.7.0. - Validate NativeAOT/trimming compatibility on `net10.0` for the default Dekaf backend; the opt-in Confluent backend remains JIT-only. - Avoid per-header allocation on .NET Framework when Dekaf header memory is array-backed. Validation: - Release builds passed with zero warnings for all six library target frameworks. - Kafka adapter and DI selection tests passed on `net48` and `net10.0`. - `dotnet publish tests\Opc.Ua.Aot.Tests\Opc.Ua.Aot.Tests.csproj -c Release -r win-x64 --self-contained true` - The generated NuGet package includes Dekaf 1.7.0 and Confluent.Kafka 2.15.0 for every target framework. ## 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. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received.	2026-07-23 10:18:11 +02:00
Marc Schier	39991bfea1	Harden managed subscription recreation and reapply (#4065 ) ## Summary - Makes managed subscription recreation generation-safe by quiescing publish ingress, draining in-flight requests, retiring queued messages and acknowledgements from the old server-side generation, then recreating monitored items as one serialized operation. - Exposes `RecreateAsync` on `ISubscription`, rejects recreate/dispose reentrancy from notification callbacks, preserves the active generation when server-side deletion fails, and coordinates disposal with an in-flight recreate. - Aggregates logical subscription recovery state across partitions and adds pending/reapply APIs so monitored items can be deterministically re-created after model or configuration changes. - Initializes the Quickstarts Boiler logger with `NullLogger` so simulation updates remain safe before a logger is assigned. - Keeps the new logical recovery test compatible with net48. ## Review follow-up - Documents the `RecreateAsync` callback-reentrancy and disposal exceptions on the public interface. - Extends generation-retirement coverage with a 1,024-message stale burst and verifies that only ordered current-generation callbacks are delivered. - Proves manager disposal cancels a cancellation-aware operation holding publish quiescence and waits for the gate to release safely. - No runtime timeout or blanket channel drain was added: the quiesced production delegate is internal and cancellation-aware, while unconditional draining could discard or reorder current-generation messages arriving through direct ingress. ## Validation Current head: `edb4a75145c8b03bd03da15eb71327e3bffda091`. Review follow-up validation on the current head: - `Opc.Ua.Client` and `Quickstarts.Servers` build successfully for net472, net48, netstandard2.1, net8.0, net9.0, and net10.0. - focused `SubscriptionTests` and `SubscriptionManagerTests`: 72 passed on net10.0 and 72 passed on net48. The runtime implementation is unchanged from `7b944ac68616e2d6c0595957c65179f86e78a637`, which passed: - net10.0 `Opc.Ua.Client.Tests`: 1,974 passed. - net10.0 `Opc.Ua.Sessions.Tests`: 738 passed, 379 skipped. - net10.0 `Opc.Ua.Subscriptions.Tests`: 601 passed, 6 skipped. - focused net48 managed-subscription tests: 122 passed. - win-x64 Native-AOT publish succeeded; subscription, monitored-item, and Boiler AOT cohorts: 14 passed.	2026-07-23 09:54:26 +02:00
Marc SchierandCopilot	dd6a591ebe	Enforce NodeManagement authorization and rollback (OPC 10000-4/18) (#4046 ) ## Summary - Fixes missing `AddNode`, `DeleteNode`, `AddReference`, and `RemoveReference` authorization before NodeManagement mutations. - Enforces applicable namespace and Node `AccessRestrictions`. - Distinguishes local targets from remote `targetServerUri`, server-index, and namespace-URI targets. - Validates both local reference endpoints and uses the actual reciprocal `NodeClass` for cross-NodeManager mutations. - Adds bounded, cancellation-safe compensation when reciprocal add/delete mutations fail. ## Specification - OPC 10000-4 §§5.8.2-5.8.3 - OPC 10000-18 role-based access control This is split from #4021. ## Tests Focused Release validation passed on both `net10.0` and `net48`: - `dotnet build tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f <TFM> -p:CustomTestTarget=<TFM>` - `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f <TFM> -p:CustomTestTarget=<TFM> --no-build --no-restore --filter FullyQualifiedName~Opc.Ua.Server.Tests.MasterNodeManagerNodeManagementTests` - 55 tests passed per TFM, including `AddNodesDeniedAddNodePermissionDoesNotMutateAsync`, `AddNodesDeniedParentAddReferencePermissionDoesNotMutateAsync`, `DeleteNodesDeniedDeleteNodePermissionDoesNotMutateAsync`, `AddReferencesGrantedPermissionsMutateBothSidesAsync`, `AddReferencesTargetServerUriSkipsLocalTargetProcessingAsync`, `AddReferencesInverseFailureRollsBackSourceAsync`, `AddReferencesCancellationUsesIndependentRollbackToken`, `DeleteReferencesRemoteTargetSkipsReciprocalProcessingAsync`, `DeleteReferencesInverseFailureRestoresSourceAsync`, and `DeleteReferencesUnexpectedFailureRestoresSourceWithIndependentToken`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-23 09:19:11 +02:00
Copilot Marc Schier Marc Schier	f95d65754d	Register mandatory opc.tcp transport in AddOpcUa (make AddOpcTcpTransport optional) (#4074 ) # Description `AddOpcUa()` did not register an `ITransportBindingRegistry`. The client resolves it with `GetRequiredService(...)`, so a client built with `AddOpcUa().AddClient(...)` threw during managed-session creation unless `AddOpcTcpTransport()` was also called. (Servers tolerated a missing registry via `ServerBase`'s lazy `WithDefaultTcp()` fallback, which is why `MinimalBoilerServer` worked but `MinimalClient` did not.) This change makes the mandatory `opc.tcp` transport always available while keeping it overridable and removable, then reverts the sample workaround. ## Changes - `AddOpcUa()` seeds the mandatory transport. `AddTransportBindingRegistry()` now seeds the registry with the raw-socket `opc.tcp` secure-channel listener and tcp connection channel factories before running the registered `ITransportBindingConfigurator`s, and `AddOpcUa()` calls it. `AddOpcTcpTransport()` is now optional (kept for backward compatibility). - Overridable. Because the seed runs before configurators, `AddKestrelOpcTcpTransport()` / `AddHttpsTransport()` / `AddWssTransport()` / `AddCustomTransport<,>()` still override the seeded defaults last-writer-wins per URI scheme — now without needing `AddOpcTcpTransport()` first. - Removable. Added `RemoveListenerFactory(scheme)` / `RemoveChannelFactory(scheme)` to `ITransportBindingRegistry` (+ `DefaultTransportBindingRegistry`) so an implementer can unregister the seeded defaults from a plain `ITransportBindingConfigurator` — no fluent API required. - Sample. Reverted the `MinimalClient` `.AddOpcTcpTransport()` workaround; it now connects with just `.AddOpcUa().AddClient(...)`. - Small note in `docs/Transports.md` that `AddOpcTcpTransport()` is optional. ## Tests / validation - New/updated unit tests in `TransportBindingRegistryTests`: mandatory-`opc.tcp`-by-default, override-default-without-`AddOpcTcpTransport`, implementer-can-unregister-default, and `Remove` unit tests; updated `ParallelServiceProvidersHaveIndependentRegistries` for the seeded default. - Client DI regression test in `ClientDiFluentApiTests` (`AddOpcUa().AddClient(...)` resolves the registry + channel manager without an explicit tcp transport). - Updated the `SpyTransportBindingRegistry` test double for the two new interface members. - End-to-end:* started `MinimalBoilerServer` and ran `MinimalClient --auto-accept` → discovered a `SignAndEncrypt` / `Basic256Sha256` endpoint, connected, browsed the Objects folder, read `ServerStatus.CurrentTime`, and created a subscription (exit code 0) with no `AddOpcTcpTransport()`. - Green locally: `Opc.Ua.Core.Tests` (4123 passed), `Opc.Ua.Client.Tests` (1962 passed), `Opc.Ua.Server.Tests` Hosting (141 passed) on net10.0; transport + client-DI tests on net48; `Opc.Ua.Core` builds with 0 warnings. ## Related Issues Tracked by the linked issue. ## 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>	2026-07-23 07:26:19 +02:00
Jonas	142a9a40a6	Fix MCP startup failure and respect session name (#4077 ) # Description Fixes a Microsoft.Extensions.Options.OptionsValidationException when starting the MCP server and fixes the Connection tools to return and use the correct session name. ## Related Issues - Fixes #4076 ## 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. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received.	2026-07-23 07:26:03 +02:00

1 2 3 4 5 ...