2 Commits

Author SHA1 Message Date
romanett 668310f21f Make the fluent node manager's configure/seal path genuinely async (#4442)
# Description

The fluent node-manager pipeline had two synchronous chokepoints, and
two independent efforts have now hit both of them.

`Configure` is a `partial void`, so it cannot `await`. A manager whose
wiring depends on asynchronous setup had to invent a second hook outside
the pipeline, create its own builder there, and hand-roll
`CreateFluentBuilder(...).Configure(...).Seal()` inside it.

`Seal()` was synchronous, so nothing a registration staged during
`Configure` could be completed by awaiting. Registrations that needed to
await either blocked — `EventSourceRegistry` did
`.GetAwaiter().GetResult()` on the manager's root-notifier registration
— or could not be expressed at all.

## What changed

**`FluentNodeManagerBase.ConfigureAsync(INodeManagerBuilder,
CancellationToken)`** is the awaitable wiring seam. It runs once per
activation, with the manager's attached builder, immediately *before*
the synchronous `Configure` partial(s) — so instances it materialises
are in the address space by the time `Configure` wires callbacks against
them. The default implementation is a no-op.

It is a `virtual` method rather than a second `partial` declaration
because a partial method can be optional (`partial void`, no return
value) or awaitable (an extended partial method, which must be
implemented) — not both, and the generator would otherwise have to
detect which one the user declared. `partial void
Configure(INodeManagerBuilder)` keeps working unchanged; a manager uses
either hook or both.

**`NodeManagerBuilder.Seal()` becomes `SealAsync(CancellationToken)`.**
Sealing is now the single point where registrations that could not
finish inside `Configure` get their turn to await. Every seal site
awaits it, so a later change can move registry *setup* onto the same
seam.

**`DiNodeManager` now runs the standard pipeline itself** — create and
attach the builder, `await ConfigureAsync`,
`RegisterAuthoredNodesAsync`, `CompleteConfigureAsync`, the post-setup
runner, then `await builder.SealAsync` — instead of handing subclasses a
bare callback and leaving them to build and seal on their own. DI
subclasses gain node registration and the reverse-reference pass, which
they previously skipped.

Sealing deliberately comes **after** the post-setup runner. The fluent
registries are owned by the manager, not by a single builder, and
sealing starts them — so sealing first locks post-setup configurators
out of registering simulation loops of their own, which is exactly what
`AddRobotics`' configurators do. Sealing before the runner makes
`MinimalRobotServer` fail to start with *"Cannot add a simulation loop
after the registry has started"*. Worth knowing for follow-up work: with
several builders attached to one manager, the first seal freezes the
shared registry for all of them.

## Workarounds removed

- **`DiNodeManager.OnAddressSpaceReadyAsync`** — deleted. Its three
overriders (`RoboticsNodeManager`, and the `PumpNodeManager` /
`GeneratorNodeManager` samples) now override `ConfigureAsync` and no
longer hand-roll `CreateFluentBuilder(...).Configure(Configure).Seal()`.
- **`EventSourceRegistry`'s `.GetAwaiter().GetResult()`** on
`AddRootNotifierAsync` — gone. `Publish(..., RegisterAsRootNotifier:
true)` now stages the registration and `SealAsync` drains it, so the
wait happens on an awaited path. A source whose root-notifier
registration fails is rolled out of the registry before the failure
surfaces, and a registry disposed before its builder sealed drops what
it staged.
- **`Isa95NodeManager.ConfigureStatusEvents()`** — the synchronous
second builder pass is now
`ConfigureStatusEventsAsync(CancellationToken)`, awaited from
`CreateAddressSpaceAsync`. (`ConfigureCatalogChanges()` stays
synchronous: it starts a background task and never seals a builder, so
there was nothing to convert.)
- **`IRoboticsBuildContext.Seal()`** — now
`SealAsync(CancellationToken)`. These really were
`NodeManagerBuilder.Seal()` calls behind a same-named method. The state
transition still happens under the context lock; the builder's
asynchronous completion runs outside it, so no `await` is taken while
the lock is held.
- The Pump / Generator incremental registration callbacks
(`onRegistered`) changed from `Action<T>` to `Func<T, CancellationToken,
ValueTask>` so their per-instance builder pass can await its own seal.

No sync-over-async was introduced anywhere. `Dispose(bool)` is unchanged
and stays non-blocking; teardown remains in `DeleteAddressSpaceAsync`.

`NodeManagerBuilder.Seal()` and `IRoboticsBuildContext.Seal()` are
removed rather than kept as synchronous overloads. A synchronous seal
that silently skipped the asynchronous completion would reintroduce
exactly the failure mode this change exists to remove.

## Generator

`NodeManagerTemplates` emits, in order: predefined nodes load → `await
ConfigureAsync(__m_builder, cancellationToken)` →
`Configure(__m_builder)` and the typed `Configure` →
`RegisterAuthoredNodesAsync` → `CompleteConfigureAsync` → `await
SealConfigurationAsync(__m_builder, cancellationToken)`.

`NodeManagerGeneratorTests` asserts that order literally, and that
neither `__m_builder.Seal()` nor the synchronous
`SealConfiguration(__m_builder)` is emitted.

### Merging with #4432

#4432 landed on `master` while this was in review and reworked the same
seam from the other side: it split `Seal()` into `SealGraphAuthoring()`
+ `StartSimulations()` so a manager can replay `NotifyNodeAdded`
*between* the two halves — sealing first stops a lifecycle handler
authoring nodes nothing would register, starting the simulations last
stops a simulated value change preceding its own node''s `OnNodeAdded` —
and added `FluentNodeManagerBase.SealConfiguration(builder)` as the one
call the generator emits.

Both intents are kept. That split is precisely why the asynchronous
completion could not simply live inside `SealAsync`: a manager reaching
the seam through the split path would silently skip it, which is the
failure mode this PR exists to remove. So the asynchronous half became
its own step:

- `NodeManagerBuilder.CompleteSealAsync(ct)` drains the staged
root-notifier registrations, then starts the simulations.
`SealAsync(ct)` is `SealGraphAuthoring()` followed by it.
- #4432''s synchronous `StartSimulations()` is gone, folded into
`CompleteSealAsync`. It had exactly one production caller after the
merge, and leaving it standing would have kept a second, synchronous way
to activate a builder that skips the staged registrations — the very
hazard this PR removes. Activation is now one step with one entry point;
the *split* itself stays, because the replay has to sit between the
halves.
- `SealConfiguration` becomes `SealConfigurationAsync(builder, ct)`:
seal the graph, replay `NotifyNodeAdded`, then `CompleteSealAsync`.
Activation still happens last, so #4432''s ordering guarantee holds
unchanged.

## Consumers unblocked

- **#4434 ("Reimplement the GDS ApplicationsNodeManager as a generated
fluent node manager")** adds `OnAddressSpaceReadyAsync` to
`FluentNodeManagerBase` purely because, in its own words, *"`Configure`
is a `partial void` and cannot await"*. That hook is unnecessary once
`ConfigureAsync` exists — the GDS manager overrides `ConfigureAsync`
instead, and the ordering it wants (predefined nodes → async setup →
`Configure`) is what the generator now emits. #4434 has not merged yet,
so it can drop its addition rather than have it simplified afterwards.
- **Branch `romanett/fluent-node-behaviors`** moved
`SimulationRegistry`, `EventSourceRegistry`, `MonitoredSourceRegistry`
and the alarm wiring onto `AttachToType`/`Attach` for *release* only,
because activation ran from `CompleteConfigureAsync` and most managers
only called the synchronous `Seal()`. Every seal site now reaches
`SealAsync`, so registry and alarm *setup* can move onto behavior
activation too — the remaining half of that work.

## Tests

- New `DiNodeManagerConfigureAsyncTests` drives the real
`DiNodeManager.CreateAddressSpaceAsync` and asserts the hook runs once
with an attached builder, that nodes it stages are registered, that
their references to externally owned nodes are published, and that the
builder is sealed before `CreateAddressSpaceAsync` returns.
- `PublishTests` now pins the root-notifier deferral contract:
registration is staged during `Configure`, lands only on the seal, and
draining twice does not double-register.
- Every `Seal()` call site in the test suites became `await
...SealAsync()`, including the three #4432 added in
`NodeSetImportBuilderTests` and `SimulationBuilderExtensionsTests`.

## Related Issues

No tracking issue was opened for this; it is the shared prerequisite
extracted from #4434 and branch `romanett/fluent-node-behaviors`, both
of which had independently worked around the same synchrony. Happy to
open one if maintainers want the ADR trail.

## Checklist

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

### What was actually run locally

`UA.slnx` builds clean. Post-merge on **net10.0**: `Opc.Ua.Server.Tests`
5020/0 and `Opc.Ua.SourceGeneration.Core.Tests` 3813/0. Pre-merge the
same run plus `Opc.Ua.Di.Tests` 425/0, `Opc.Ua.Robotics.Tests` 627/0,
`Opc.Ua.ISA95.Tests` 136/0, `Opc.Ua.Positioning.Tests` and
`Opc.Ua.Robotics.Intent.Tests` were all green; `MinimalRobotServer` was
also started directly to confirm the hosted robotics path comes up.

Three caveats, so the unchecked box above is not a mystery: the full
suite was **not** run against .NET Framework locally;
`Opc.Ua.OpenUsd.Tests` has one pre-existing failure on this machine
(`StructuredCartesianCoordinatesAreAccepted` expects `"1.5"` and gets
`"1,5"` — a German-locale decimal separator, unrelated to this change);
and one pre-existing `CA1861` warning comes from
`NodeSetImportIntegrationTests.cs`, which is byte-identical to `master`
on this branch.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-09-08 12:01:56 +00:00
Copilot d525bc3f28 Bound reconnect cleanup and organize samples by domain (#4276)
# Description

This PR fixes bounded ManagedSession reconnect cleanup and reorganizes
the sample applications into domain-focused folders.

## Reconnect cleanup

- Bounds reconnect integration cleanup with independent 30-second close
and dispose limits.
- Sends `CloseSession` through the awaited transport request path during
`Session.DisposeAsync`.
- Preserves explicit `Session.CloseAsync` behavior that waits for or
cancels outstanding publish requests.
- Covers the disposal request path with a regression test.

## Sample organization

- `samples/OpenUsd`: `GeneratorServer`, `SiteCompositionServer`
- `samples/Redundancy`: `RedundantClient`, `RedundantServer`,
`RedundantPubSub`
- `samples/Isa95`: `MinimalIsa95Server`
- `samples/MinimalApi`: `MinimalBoilerServer`, `MinimalCalcServer`,
`MinimalClient`
- `samples/DI`: `PumpDeviceIntegrationServer`
- `samples/Lds`: `ConsoleLdsServer`
- `samples/Reference`: `ConsoleReferenceClient`,
`ConsoleReferenceServer`
- `samples/PubSub`: `ConsoleReferencePubSubClient`

`UA.slnx` now exposes the same requested domain groups. Project
references, tests, runtime sample probes, CI workflows, Dockerfiles,
Compose files, helper scripts, and documentation were updated to the new
paths. `samples/Robotics/MinimalRobotServer` remains under `Robotics`.

## Validation

- `dotnet build UA.slnx`
- 3 Redundancy short-haul sample tests
- 70 DI sample tests
- 914 OpenUSD tests
- 1 WotCon model-materialization test
- 137 AOT tests
- `docker compose config` for all 6 moved Redundancy Compose files

## Related Issues

- Fixes #4273

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce new build or
analyzer warnings.
- [x] I built the complete `UA.slnx` solution locally.
- [ ] I ran all tests locally against at least .NET Framework and .NET
10.
- [ ] I fixed all failing and flaky tests in the CI pipelines and all
CodeQL warnings.
- [x] I have addressed all PR feedback received.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <mschier@microsoft.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Copilot-Session: a2c2838f-74ae-4997-a1d4-50574d9e0636
2026-08-21 04:35:47 +00:00