Make the fluent node manager's configure/seal path genuinely async - #4442
Conversation
`Configure` is a `partial void` and cannot await, and `Seal()` was synchronous, so nothing a registration staged during `Configure` could be completed by awaiting. Managers whose wiring needed asynchronous setup had to invent a hook outside the pipeline and hand-roll `CreateFluentBuilder(...).Configure(...).Seal()` inside it; registrations that needed to await had to block instead. Add `FluentNodeManagerBase.ConfigureAsync(INodeManagerBuilder, CancellationToken)` as the awaitable wiring seam. It runs once per activation with the manager's attached builder, immediately before the synchronous `Configure` partial(s), so instances it materialises are in the address space by the time `Configure` wires callbacks against them. It is a virtual method rather than a second partial declaration because a partial method can be optional (`partial void`) or awaitable (extended partial, which must be implemented) — not both; `partial void Configure` keeps working unchanged. Replace `NodeManagerBuilder.Seal()` with `SealAsync(CancellationToken)`. Sealing is now the single point where registrations that could not finish inside `Configure` get their turn to await, and every seal site awaits it. Workarounds removed: - `DiNodeManager.OnAddressSpaceReadyAsync` — deleted. `DiNodeManager` runs the standard pipeline itself and its three overriders (RoboticsNodeManager, the Pump and Generator samples) override `ConfigureAsync` instead. Sealing comes after the post-setup runner because the fluent registries are owned by the manager, not by one builder, and sealing starts them: sealing first locks post-setup configurators out of registering simulation loops. - `EventSourceRegistry`'s `.GetAwaiter().GetResult()` on `AddRootNotifierAsync` — `Publish(..., RegisterAsRootNotifier: true)` now stages the registration and `SealAsync` drains it. - `Isa95NodeManager.ConfigureStatusEvents()` — its second builder pass is now async and awaited from `CreateAddressSpaceAsync`. - `IRoboticsBuildContext.Seal()` — now `SealAsync`; the state transition stays under the context lock, the builder's completion runs outside it. The generator emits, and its ordering tests assert: predefined nodes load → `ConfigureAsync` → `Configure` → `RegisterAuthoredNodesAsync` → `CompleteConfigureAsync` → `SealAsync` → `NotifyNodeAdded` replay. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves against #4432 (NodeSet2 import into a fluent node manager), which split `Seal()` into `SealGraphAuthoring()` + `StartSimulations()` so a manager can replay `NotifyNodeAdded` between the two, and added `FluentNodeManagerBase.SealConfiguration(builder)` as the single entry point the generator emits. Both intents are kept. The split stays, and the asynchronous half of sealing becomes its own step so managers reaching it through either path still run it: - `NodeManagerBuilder.CompleteSealAsync(ct)` drains the registrations `Configure` could not await (root notifiers) and then starts the simulations. `SealAsync(ct)` is `SealGraphAuthoring()` followed by it. - `FluentNodeManagerBase.SealConfiguration` becomes `SealConfigurationAsync(builder, ct)`: seal the graph, replay `NotifyNodeAdded`, then `CompleteSealAsync`. Activation still happens last, so no simulated value change precedes the `OnNodeAdded` handler of its own node, and the replay still cannot author nodes. - The generator emits `await SealConfigurationAsync(__m_builder, cancellationToken)`; its ordering test asserts the merged sequence (`ConfigureAsync` -> `Configure` -> `RegisterAuthoredNodesAsync` -> `CompleteConfigureAsync` -> `SealConfigurationAsync`) and that neither `Seal()` nor the synchronous `SealConfiguration(...)` is emitted. - `RuntimeNodeSetNodeManager` takes master's `SealConfiguration` call in its awaited form. The three `Seal()` call sites master added in `NodeSetImportBuilderTests` and `SimulationBuilderExtensionsTests` became `await ...SealAsync()`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
After merging #4432 the builder had two ways to finish a seal: the asynchronous `CompleteSealAsync`, which drains the registrations `Configure` could not await and then starts the simulations, and the synchronous `StartSimulations`, which only did the latter. `StartSimulations` had exactly one production caller left (`CompleteSealAsync` itself). Keeping it standing would have preserved a second, synchronous way to activate a builder that silently skips the staged root-notifier registrations — the same hazard that motivated replacing `Seal()` with `SealAsync` in the first place. Fold it in, so activation is one step reached through one entry point. The split #4432 introduced stays: `SealGraphAuthoring()` closes graph authoring, the manager replays `NotifyNodeAdded`, and `CompleteSealAsync()` activates. The replay has to sit between the two halves, which a single `SealAsync` cannot express, so both halves remain genuinely necessary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code coverage✅ Coverage gate passed.
Uncovered changed lines
Coverage is above the recorded baseline - consider ratcheting Thresholds live in |
There was a problem hiding this comment.
🔵 Needs a closer look
EventSourceRegistry.CompleteRegistrationsAsync currently wraps cancellation into BadConfigurationError, which can break correct CancellationToken propagation during async sealing.
Pull request overview
This pull request updates the fluent node-manager activation pipeline to be genuinely asynchronous by introducing an awaitable configuration hook and converting builder sealing into an awaited async step, then wiring DI managers, runtime managers, samples, docs, and tests to the new flow.
Changes:
- Add
FluentNodeManagerBase.ConfigureAsync(INodeManagerBuilder, CancellationToken)as an awaitable wiring seam executed before synchronousConfigure(...). - Replace synchronous sealing with awaited sealing (
NodeManagerBuilder.SealAsync(...)+ split sealing/activation viaSealGraphAuthoring()andCompleteSealAsync(...)), and update all call sites. - Update DI node-manager activation so it consistently runs the full fluent pipeline (configure/register/complete/post-setup/seal), plus update docs and tests to pin ordering and contracts.
File summaries
| File | Description |
|---|---|
| tools/Opc.Ua.SourceGeneration.Core/Generators/NodeManagerTemplates.cs | Generator now emits await ConfigureAsync(...) before Configure(...) and awaits SealConfigurationAsync(...). |
| tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeManagerGeneratorTests.cs | Updates contract assertions for the new async configure + async seal emission order. |
| tests/Opc.Ua.Server.Tests/Fluent/VirtualNodeBuilderTests.cs | Switches builder sealing to await SealAsync() and updates affected tests to async. |
| tests/Opc.Ua.Server.Tests/Fluent/SimulationBuilderExtensionsTests.cs | Updates simulation sealing tests for the seal/activation split and async sealing. |
| tests/Opc.Ua.Server.Tests/Fluent/RuntimeValueBuilderExtensionsTests.cs | Uses awaited SealAsync() to reflect new sealing semantics. |
| tests/Opc.Ua.Server.Tests/Fluent/PublishTests.cs | Pins root-notifier registration deferral to the seal completion step and adds idempotency coverage. |
| tests/Opc.Ua.Server.Tests/Fluent/NodeSetImportBuilderTests.cs | Converts seal-related assertions to async sealing. |
| tests/Opc.Ua.Server.Tests/Fluent/NodeManagerBuilderTests.cs | Converts post-seal behavior checks to async sealing. |
| tests/Opc.Ua.Server.Tests/Fluent/NodeManagerBuilderAuthoringTests.cs | Updates the authoring pipeline test harness to await sealing. |
| tests/Opc.Ua.Server.Tests/Fluent/MonitoredItemFluentTests.cs | Switches helper initialization from Seal() to awaited SealAsync(). |
| tests/Opc.Ua.Server.Tests/Fluent/GeneratedManagerHybridTests.cs | Updates hybrid wiring-sequence test to await sealing. |
| tests/Opc.Ua.Robotics.Tests/RoboticsTopologyBuilderTests.cs | Converts build-context sealing to SealAsync(...) and updates exception assertions accordingly. |
| tests/Opc.Ua.Robotics.Tests/RoboticsServerHostingTests.cs | Updates test stub interface implementation for new async seal signature. |
| tests/Opc.Ua.Di.Tests/DiNodeManagerConfigureAsyncTests.cs | Adds coverage that DI pipeline invokes ConfigureAsync with an attached builder and seals before returning. |
| src/Opc.Ua.Server/RuntimeNodeSet/RuntimeNodeSetNodeManager.cs | Updates runtime nodeset manager to await SealConfigurationAsync(...). |
| src/Opc.Ua.Server/Hosting/FluentNodeManagerFactory.cs | Updates hosting factory to await builder.SealAsync(ct). |
| src/Opc.Ua.Server/Fluent/NodeManagerBuilder.cs | Introduces SealAsync(...) and splits sealing vs activation with SealGraphAuthoring() + CompleteSealAsync(...). |
| src/Opc.Ua.Server/Fluent/NodeManagerBuilder.Authoring.cs | Updates documentation references from Seal() to SealAsync(). |
| src/Opc.Ua.Server/Fluent/FluentNodeManagerBuilderExtensions.cs | Updates fluent pipeline docs to reflect async sealing being awaited separately. |
| src/Opc.Ua.Server/Fluent/FluentNodeManagerBase.cs | Adds the awaitable ConfigureAsync(...) seam and replaces SealConfiguration(...) with SealConfigurationAsync(...). |
| src/Opc.Ua.Server/Fluent/EventSourceRegistry.cs | Defers root-notifier registration to an awaited drain step invoked during sealing. |
| src/Opc.Ua.Robotics.Server/RoboticsNodeManager.cs | Moves DI robotics manager customization to the new ConfigureAsync(...) seam. |
| src/Opc.Ua.Robotics.Server/RoboticsBuildContext.cs | Converts robotics build context sealing to SealAsync(...) without awaiting under the context lock. |
| src/Opc.Ua.Robotics.Server/IRoboticsBuildContext.cs | Updates the build-context interface to async sealing. |
| src/Opc.Ua.Robotics.Server/Hosting/OpcUaServerRoboticsBuilderExtensions.cs | Awaits context.SealAsync(...) at end of robotics configuration pipeline. |
| src/Opc.Ua.ISA95.Server/Isa95NodeManager.cs | Switches to awaited sealing and makes the status-event builder pass async so it can await sealing. |
| src/Opc.Ua.Di.Server/DiNodeManager.cs | DI manager now owns the full fluent pipeline and seals after post-setup configurators run. |
| samples/OpenUsd/SiteCompositionServer/SiteNodeManager.cs | Updates sample manager sealing to awaited SealAsync(ct). |
| samples/OpenUsd/GeneratorServer/GeneratorNodeManager.cs | Moves generator manager wiring to ConfigureAsync(builder, ct) and relies on base DI sealing. |
| samples/OpenUsd/GeneratorServer/GeneratorNodeManager.Configure.cs | Converts incremental “onRegistered” simulation wiring to async seal. |
| samples/DI/PumpDeviceIntegrationServer/README.md | Updates documentation diagrams and narrative for the new async configure + seal ordering. |
| samples/DI/PumpDeviceIntegrationServer/PumpNodeManager.cs | Moves pump DI manager wiring to ConfigureAsync(builder, ct) and relies on base DI sealing. |
| samples/DI/PumpDeviceIntegrationServer/PumpNodeManager.Configure.cs | Converts incremental “onRegistered” simulation wiring to async seal. |
| samples/DI/PumpDeviceIntegrationServer/OpenUsdRepresentation.cs | Updates comment to reference ConfigureAsync rather than the removed OnAddressSpaceReadyAsync. |
| docs/NodeManagers.md | Updates generated-manager lifecycle documentation for async configure and async sealing. |
| docs/DeviceIntegration.md | Updates DI runner ordering documentation to match the new pipeline and seal placement. |
Review details
Suppressed comments (1)
src/Opc.Ua.Server/Fluent/EventSourceRegistry.cs:203
- CompleteRegistrationsAsync catches all exceptions (including OperationCanceledException) and wraps them as BadConfigurationError. Now that SealAsync passes a CancellationToken, cancellation should propagate (and the current notifier should remain staged so a later seal attempt can retry).
catch (Exception ex)
{
lock (m_sourcesLock)
{
m_sources.Remove(notifier.NodeId);
- Files reviewed: 36/36 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The coverage report flagged the early return in `RoboticsBuildContext.SealAsync` as the one uncovered branch this change introduced. It is the idempotency contract the hosting extension relies on — the context is reachable from configurators, so a redundant seal has to be ignored rather than re-run the builder's completion work — so it is worth pinning rather than leaving to inspection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`CompleteRegistrationsAsync` caught every exception and wrapped it as `BadConfigurationError`. That was harmless while the drain ran under `CancellationToken.None`, but `SealAsync` now flows the real token in, so a cancelled seal surfaced as a configuration error and the caller could no longer tell an aborted activation from a genuinely broken `Publish` registration. Cancellation now propagates as `OperationCanceledException`, and the notifiers that were not registered yet go back to the front of the pending queue instead of being dropped, so a later seal can finish the job. The drain only rolls a source out of the registry for real failures, where a half-registered source must not survive. Found by the Copilot reviewer on #4442. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4442 +/- ##
==========================================
+ Coverage 80.82% 80.84% +0.01%
==========================================
Files 2030 2029 -1
Lines 295603 295645 +42
Branches 51030 51033 +3
==========================================
+ Hits 238931 239016 +85
+ Misses 38938 38901 -37
+ Partials 17734 17728 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
`FluentNodeManagerBuilderExtensions.Configure` existed to express `CreateFluentBuilder(ns).Configure(Configure).Seal()` as one chained expression. Sealing is awaited now, so that chain cannot be written any more, and every former caller — the Pump and Generator samples — calls `Configure(builder)` directly from its `ConfigureAsync` override. Nothing calls it: the only reference left was a `<see cref>` in `CreateFluentBuilder`'s documentation, which the coverage report made visible as the file dropping to 0%. A one-line pass-through that returns its own argument earns nothing once it cannot chain, and `Configure(builder)` is shorter than `builder.Configure(Configure)`. This is a public API removal, in the same surface and for the same reason as the `Seal()` removals this change already makes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both pipelines are ordering contracts described only in prose, and the order is the whole point of this change: what runs before the seal, what the seal itself does in halves, and why the DI manager seals after its post-setup runner rather than before. docs/NodeManagers.md gains the generated `CreateAddressSpaceAsync` flow, including the three steps inside `SealConfigurationAsync` — seal the graph, replay `NotifyNodeAdded`, activate — since the replay sitting between the halves is the part prose keeps failing to make obvious. docs/DeviceIntegration.md gains the DI flow, where the seal lands after `IDiPostSetupRunner` so configurators can still register simulation loops. Requested in review on #4442. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#4442 makes the fluent configure/seal path genuinely async, which is the change this branch was waiting on. Four conflicts, all in the seam the two efforts share. EventSourceRegistry is the one that improves. Root-notifier registration is now staged during Configure and drained by SealAsync, so the sync-over-async call this branch had to live with is gone from the registration path. The ownership tracking that lets release remove only what it added layers on top of the real await. The rollback path was itself sync-over-async and is now awaited too, so the file has none left. ISA-95 keeps an async ConfigureStatusEventsAsync that activates its behaviors and then awaits SealAsync. RuntimeNodeSetNodeManager takes master's awaited seal unchanged, since CompleteConfigureAsync already drains its registrations. Seal() no longer exists, so the four SimulationLifetimeTests call sites move to SealAsync and two of those tests become async. Not done here, deliberately: now that every seal site awaits, behavior activation could move inside SealAsync and cover every manager automatically, retiring the pending-registrations warning. That is the design change this merge unblocks, and it belongs in its own commit with its own tests rather than buried in a merge resolution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#4442 adds ConfigureAsync(INodeManagerBuilder, CancellationToken) to FluentNodeManagerBase -- the awaitable wiring seam this branch had introduced as OnAddressSpaceReadyAsync, but with the builder handed over as well, and reasoned from the same constraint: a partial method can be optional or awaitable, not both. DiNodeManager, which had invented the hook privately and which this branch had turned into an override, has already moved onto it upstream. Shipping both would leave two overlapping async seams on one base class, so OnAddressSpaceReadyAsync goes: from the base class, the generated CreateAddressSpace sequence, the generator's ordering test, the GDS manager (now an override of ConfigureAsync, which ignores the builder because its work is I/O rather than address space), and docs, where master's account is fuller than the section this branch added. #4442 also replaces Seal() with SealAsync(cancellationToken). ConfigureKeyCredentialService is host-facing and had to follow, so it becomes ConfigureKeyCredentialServiceAsync; its caller and cref move with it, along with one test and a doc example. The net effect is a smaller branch: it no longer adds an API the stack now provides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Activation only ran from CompleteConfigureAsync, which several managers never call. The stopgap was a warning when a builder sealed with registrations still pending — a way of saying "this silently did nothing" rather than making it work. #4442 made every seal site await, so activation can move to the one point every manager passes through, and the warning goes with it. Activation runs first inside CompleteSealAsync, ahead of the staged root-notifier drain and the simulation start, and that order is load-bearing in both directions. An attach callback may itself call Publish with RegisterAsRootNotifier, which only stages the notifier; the drain snapshots and clears, so activating after it would discard the registration in silence. Simulations must come last because NewSimulation is rejected once the registry has started. Nothing double-activates: DrainNodeAttachments empties the list under its lock, so the generated managers that still activate at CompleteConfigureAsync simply find nothing left to drain when they later seal. ISA-95's second builder no longer needs its own activation call, since its seal now does it. One test harness needed the type table it had been able to omit: SimulationBuilderExtensionsTests seals after registering a simulation, so its drain is no longer empty and the registry's typeTree guard fires. The guard is right; the mock was incomplete. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Description Follow-up to #4390 for distributed address-space ownership, authoritative hydration, and replica-consistent wire NodeIds. Tracks #4444. Identity implementation: `a78d59067` (Server policy/lifecycle/adapter seam) and `07faf5cfd` (replica identity module, runtime wiring, examples and proofs). ## Replica identity - `UseReplicaNodeIdentity` / `ReplicaNodeIdFactory` reserve fixed shared namespace indexes before node-manager construction and reuse `DefaultNodeIdFactory`. ApplicationUri and built-in diagnostics/configuration remain local. - Replicated startup requires an explicit identity contract. Namespace layout, deterministic mode, replica-set identity and writer policy must agree. Standard factory views, replacement and registration preserve the policy. - Active/passive writer-assigned identities are replicated unchanged. Independent creation requires stable keys or explicit IDs. Live IDs and retained tombstones are reserved before writer allocation; no network/storage call runs in `New`. - Protected store descriptors use strong routing, including customized hybrid prefixes. Unknown legacy state and empty eventual scans cannot authorize automatic adoption. Verified-new hybrid provisioning is explicit. - Active/active frames compare the descriptor before CRDT merge. Rejected frames are not reported as applied; incompatible admission remains at ServiceLevel 0 through the standard service-level wiring. - Shared metadata validation covers type definitions, data types, references, permissions, NodeId/ExpandedNodeId/QualifiedName values, arrays/matrices, DataValues and encodeable structures/arguments, without reflection. - Runtime preparation hydrates retained IDs before replacement routes publish. Rebinding keeps the existing store/election or gossip transport/map and attaches to the new node-manager sources. Normal node-manager mutations, including AddNodes, reach the cached local-address-space adapter. - WoT materialization retains the stable configured factory. Samples expose genuinely factory-created nodes and a client `--identity` failover workflow. ## Preserved HA repairs The published `d99995003` and `79da74f14` fixes remain in place: - Shared linearizable sequence/lease coordination; no pretend CAS on CRDT and no private counter substituted for shared coordination. - Strict authoritative record decoding, protected pending reservations, safe snapshot publication and required delta retention. - Accepted topology state controls authoritative membership; stale deletion cannot remove a newer incarnation. - Independent subtree attachment tracking and correct disposal of the final snapshot task. - Values wait for their topology, and parent tombstone horizons prevent older descendant values crossing incarnations. A new deterministic regression also covers writer hydration preparation: temporary reconciliation removals must remain inbound application, not be recaptured as new shared deletion records. ## Validation ### Review feedback fixes `3a682155b` addresses the active-node notification, WoT namespace-index bounds, and retired-manager XML documentation feedback. The address-space adapter now notifies listeners with the registered active node, and both WoT managers reject missing/out-of-range namespace indexes before ownership is established. Targeted feedback validation passes on net10.0 and net48: 8 address-space adapter cases and 74 WoT cases per framework, plus 17 hydration/gossip/live-client integration cases on net10.0. The wrong-instance notification and missing namespace regressions were reproduced before the fixes. ### Identity implementation evidence | Selected scope | net10.0 | net48 | | --- | --- | --- | | HA repair, identity, metadata, peer admission and real-client identity/reload fixtures | 215 passed | 215 passed | | Factory, fluent authoring/import, NodeManagement and address-space adapter fixtures | 213 passed | 213 passed | | WoT node-manager fixtures | 68 passed | 68 passed | - Real UA-TCP clients compare full NodeIds/reference targets across independent servers in all four deterministic modes, with different manager/creation order and local allocations. Saved IDs work after endpoint failover and restart. - Real AddNodes-created IDs remain readable through successive live manager replacements in both replication modes, including nodes created after a reload. - Two process-level cases start three sample replicas, kill the serving process, verify a different serving ServerArray URI, and reuse cached factory-generated IDs for Read and CreateMonitoredItems without remapping or rebrowsing. - The identity/hydration scenario runs from the actual published win-x64 NativeAOT executable in the existing historian companion host. - The focused writer-recapture regression was observed failing without its fix, then passing. The real reload scenario was additionally repeated while diagnosing that race. - Changed-file formatting/style/analyzers were applied. Existing CA1508, CA1823, CA1861 and CA1850 warnings in untouched upstream files were not suppressed. The selected checks above do not replace full-solution CI, coverage gates or maintainer review, and do not claim that the entire solution or every native platform was run. #4444 remains the tracking issue. ## Contract and consistency limits Fixed identity does not make eventual payload reads linearizable. Hybrid views remain non-destructive, use explicit tombstones, and do not infer empty-store bootstrap or partition completion from absence. Compacted snapshots require authoritative storage. Failed/uncertain reservations require reconciliation. There is no automatic namespace-slot allocation, persistent logical-key mapping database, live renumbering, legacy-store wipe/adoption, or new hash algorithm. Opaque shared structures require a registered codec so their namespace-bearing fields can be validated. Application callbacks are still application-owned. ## Related issues - Tracks #4444. - Follow-up to #4390. - Builds on the merged NodeId factory (#4433), configure/seal (#4442) and lifecycle/import (#4427) integration at `aedd3f738`. ## Checklist - [ ] I have signed the CLA and read CONTRIBUTING. - [x] Added targeted regression and integration tests. - [x] Updated directly related API, HA, migration and sample documentation. - [x] Ran scoped formatting and the targeted framework checks listed above. - [ ] Full UA.slnx matrix and coverage gates completed. - [ ] All CI and maintainer review completed. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dbd9527-d818-47a0-984c-fbb1472d3d6b
Description
The fluent node-manager pipeline had two synchronous chokepoints, and two independent efforts have now hit both of them.
Configureis apartial void, so it cannotawait. A manager whose wiring depends on asynchronous setup had to invent a second hook outside the pipeline, create its own builder there, and hand-rollCreateFluentBuilder(...).Configure(...).Seal()inside it.Seal()was synchronous, so nothing a registration staged duringConfigurecould be completed by awaiting. Registrations that needed to await either blocked —EventSourceRegistrydid.GetAwaiter().GetResult()on the manager's root-notifier registration — or could not be expressed at all.What changed
FluentNodeManagerBase.ConfigureAsync(INodeManagerBuilder, CancellationToken)is the awaitable wiring seam. It runs once per activation, with the manager's attached builder, immediately before the synchronousConfigurepartial(s) — so instances it materialises are in the address space by the timeConfigurewires callbacks against them. The default implementation is a no-op.It is a
virtualmethod rather than a secondpartialdeclaration because a partial method can be optional (partial void, no return value) or awaitable (an extended partial method, which must be implemented) — not both, and the generator would otherwise have to detect which one the user declared.partial void Configure(INodeManagerBuilder)keeps working unchanged; a manager uses either hook or both.NodeManagerBuilder.Seal()becomesSealAsync(CancellationToken). Sealing is now the single point where registrations that could not finish insideConfigureget their turn to await. Every seal site awaits it, so a later change can move registry setup onto the same seam.DiNodeManagernow runs the standard pipeline itself — create and attach the builder,await ConfigureAsync,RegisterAuthoredNodesAsync,CompleteConfigureAsync, the post-setup runner, thenawait builder.SealAsync— instead of handing subclasses a bare callback and leaving them to build and seal on their own. DI subclasses gain node registration and the reverse-reference pass, which they previously skipped.Sealing deliberately comes after the post-setup runner. The fluent registries are owned by the manager, not by a single builder, and sealing starts them — so sealing first locks post-setup configurators out of registering simulation loops of their own, which is exactly what
AddRobotics' configurators do. Sealing before the runner makesMinimalRobotServerfail to start with "Cannot add a simulation loop after the registry has started". Worth knowing for follow-up work: with several builders attached to one manager, the first seal freezes the shared registry for all of them.Workarounds removed
DiNodeManager.OnAddressSpaceReadyAsync— deleted. Its three overriders (RoboticsNodeManager, and thePumpNodeManager/GeneratorNodeManagersamples) now overrideConfigureAsyncand no longer hand-rollCreateFluentBuilder(...).Configure(Configure).Seal().EventSourceRegistry's.GetAwaiter().GetResult()onAddRootNotifierAsync— gone.Publish(..., RegisterAsRootNotifier: true)now stages the registration andSealAsyncdrains it, so the wait happens on an awaited path. A source whose root-notifier registration fails is rolled out of the registry before the failure surfaces, and a registry disposed before its builder sealed drops what it staged.Isa95NodeManager.ConfigureStatusEvents()— the synchronous second builder pass is nowConfigureStatusEventsAsync(CancellationToken), awaited fromCreateAddressSpaceAsync. (ConfigureCatalogChanges()stays synchronous: it starts a background task and never seals a builder, so there was nothing to convert.)IRoboticsBuildContext.Seal()— nowSealAsync(CancellationToken). These really wereNodeManagerBuilder.Seal()calls behind a same-named method. The state transition still happens under the context lock; the builder's asynchronous completion runs outside it, so noawaitis taken while the lock is held.onRegistered) changed fromAction<T>toFunc<T, CancellationToken, ValueTask>so their per-instance builder pass can await its own seal.No sync-over-async was introduced anywhere.
Dispose(bool)is unchanged and stays non-blocking; teardown remains inDeleteAddressSpaceAsync.NodeManagerBuilder.Seal()andIRoboticsBuildContext.Seal()are removed rather than kept as synchronous overloads. A synchronous seal that silently skipped the asynchronous completion would reintroduce exactly the failure mode this change exists to remove.Generator
NodeManagerTemplatesemits, in order: predefined nodes load →await ConfigureAsync(__m_builder, cancellationToken)→Configure(__m_builder)and the typedConfigure→RegisterAuthoredNodesAsync→CompleteConfigureAsync→await SealConfigurationAsync(__m_builder, cancellationToken).NodeManagerGeneratorTestsasserts that order literally, and that neither__m_builder.Seal()nor the synchronousSealConfiguration(__m_builder)is emitted.Merging with #4432
#4432 landed on
masterwhile this was in review and reworked the same seam from the other side: it splitSeal()intoSealGraphAuthoring()+StartSimulations()so a manager can replayNotifyNodeAddedbetween the two halves — sealing first stops a lifecycle handler authoring nodes nothing would register, starting the simulations last stops a simulated value change preceding its own node''sOnNodeAdded— and addedFluentNodeManagerBase.SealConfiguration(builder)as the one call the generator emits.Both intents are kept. That split is precisely why the asynchronous completion could not simply live inside
SealAsync: a manager reaching the seam through the split path would silently skip it, which is the failure mode this PR exists to remove. So the asynchronous half became its own step:NodeManagerBuilder.CompleteSealAsync(ct)drains the staged root-notifier registrations, then starts the simulations.SealAsync(ct)isSealGraphAuthoring()followed by it.StartSimulations()is gone, folded intoCompleteSealAsync. It had exactly one production caller after the merge, and leaving it standing would have kept a second, synchronous way to activate a builder that skips the staged registrations — the very hazard this PR removes. Activation is now one step with one entry point; the split itself stays, because the replay has to sit between the halves.SealConfigurationbecomesSealConfigurationAsync(builder, ct): seal the graph, replayNotifyNodeAdded, thenCompleteSealAsync. Activation still happens last, so Import NodeSet2 documents into a fluent node manager #4432''s ordering guarantee holds unchanged.Consumers unblocked
OnAddressSpaceReadyAsynctoFluentNodeManagerBasepurely because, in its own words, "Configureis apartial voidand cannot await". That hook is unnecessary onceConfigureAsyncexists — the GDS manager overridesConfigureAsyncinstead, and the ordering it wants (predefined nodes → async setup →Configure) is what the generator now emits. Reimplement the GDS ApplicationsNodeManager as a generated fluent node manager #4434 has not merged yet, so it can drop its addition rather than have it simplified afterwards.romanett/fluent-node-behaviorsmovedSimulationRegistry,EventSourceRegistry,MonitoredSourceRegistryand the alarm wiring ontoAttachToType/Attachfor release only, because activation ran fromCompleteConfigureAsyncand most managers only called the synchronousSeal(). Every seal site now reachesSealAsync, so registry and alarm setup can move onto behavior activation too — the remaining half of that work.Tests
DiNodeManagerConfigureAsyncTestsdrives the realDiNodeManager.CreateAddressSpaceAsyncand asserts the hook runs once with an attached builder, that nodes it stages are registered, that their references to externally owned nodes are published, and that the builder is sealed beforeCreateAddressSpaceAsyncreturns.PublishTestsnow pins the root-notifier deferral contract: registration is staged duringConfigure, lands only on the seal, and draining twice does not double-register.Seal()call site in the test suites becameawait ...SealAsync(), including the three Import NodeSet2 documents into a fluent node manager #4432 added inNodeSetImportBuilderTestsandSimulationBuilderExtensionsTests.Related Issues
No tracking issue was opened for this; it is the shared prerequisite extracted from #4434 and branch
romanett/fluent-node-behaviors, both of which had independently worked around the same synchrony. Happy to open one if maintainers want the ADR trail.Checklist
What was actually run locally
UA.slnxbuilds clean. Post-merge on net10.0:Opc.Ua.Server.Tests5020/0 andOpc.Ua.SourceGeneration.Core.Tests3813/0. Pre-merge the same run plusOpc.Ua.Di.Tests425/0,Opc.Ua.Robotics.Tests627/0,Opc.Ua.ISA95.Tests136/0,Opc.Ua.Positioning.TestsandOpc.Ua.Robotics.Intent.Testswere all green;MinimalRobotServerwas also started directly to confirm the hosted robotics path comes up.Three caveats, so the unchecked box above is not a mystery: the full suite was not run against .NET Framework locally;
Opc.Ua.OpenUsd.Testshas one pre-existing failure on this machine (StructuredCartesianCoordinatesAreAcceptedexpects"1.5"and gets"1,5"— a German-locale decimal separator, unrelated to this change); and one pre-existingCA1861warning comes fromNodeSetImportIntegrationTests.cs, which is byte-identical tomasteron this branch.🤖 Generated with Claude Code