[Server] Decouple service-call dispatch from MasterNodeManager lifecycle coordination

[romanett]

Description

Extracts OPC UA service-call dispatch out of MasterNodeManager into a dedicated internal NodeManagerServiceDispatcher, separating it from NodeManager lifecycle coordination as agreed in the #4094 review threads (r3649968439 / r3649998212). The 7,668-line class becomes four files with explicit, documented boundary contracts:

• NodeManagerServiceDispatcher.cs (new, internal sealed) — the implementations of GetManagerHandle/Async, TranslateBrowsePaths, Browse/BrowseNext, Read, Write, Call, HistoryRead/HistoryUpdate, ConditionRefresh, all monitored-item operations (create/restore/modify/transfer/delete/set-mode), and per-request validation. Its constructor receives only the routing table, the server context, and a logger. Lifecycle mutation state (m_dynamicMutationSemaphore, m_startupShutdownSemaphoreSlim, retired-generation collections, preparing-manager tracking) stays private to MasterNodeManager, so the dispatcher cannot reach it — the boundary is enforced by the compiler, not convention.
• MasterNodeManager.cs — now lifecycle coordination and shared infrastructure only: startup/shutdown, session notifications, namespace registration, all IDynamicNodeManagerHost / ISyncNodeManagerMonitoredItemRecovery implementations, the retired-generation notification-lease machinery, and commit/replace/rollback helpers.
• MasterNodeManager.ServiceDispatch.cs (new partial) — the unchanged public service surface. Every public virtual entry point, protected helper, static validator, and obsolete wrapper keeps its exact signature, virtual-ness, and XML docs, delegating to the dispatcher.
• MasterNodeManager.NodeManagement.cs (new partial) — the AddNodes/DeleteNodes/AddReferences/DeleteReferences service family plus NodeId-based reference plumbing. These sit between the two concerns by design: they dispatch per item but serialize address-space mutation against runtime lifecycle operations, and are the only service methods allowed to take the dynamic-mutation semaphore.

The dispatcher's crossings back into MasterNodeManager are a small audited set: internal dispatch resolves handles through the owner's public virtual GetManagerHandleAsync (10 sites), so derived-class overrides keep affecting internal routing exactly as before; event paths consume the notification-dispatch lease accessors (5 sites, widened to internal); retired-generation drain is signalled through a new one-line internal helper instead of touching the observer field (2 sites); and the browse continuation-point limit is read live per call (2 sites).

Compatibility

• IMasterNodeManager is untouched; no public or protected member changes signature or loses virtual.
• The MasterNodeManagerWithLimits test-framework subclass (overrides BrowseAsync, uses ValidatePermissionsAsync, the per-item BrowseAsync, UpdateDiagnostics, Server) passes unchanged.
• Reflection-dependent tests keep working: m_maxContinuationPointsPerBrowse remains a private field on the type and is read live on every call; PreHydrateMonitoredItemQueuesAsync remains as a private delegating method.
• Reviewable API-surface deltas, both additive: six protected static validators became protected internal static (same pattern as the existing ValidateRolePermissions) so the dispatcher can call them, and the three notification-lease nested types went private → internal.

Reviewing tip: git diff --color-moved=dimmed-zebra renders the two move commits as almost entirely dimmed blocks; the non-move edits are confined to the audited call sites above and the delegation stubs. Grepping the dispatcher for IDynamicNodeManagerHost|m_dynamicMutationSemaphore|m_startupShutdownSemaphoreSlim|m_retiredGeneration|m_preparingNodeManagers matches only its own boundary-contract doc comment.

Verification

• Full UA.slnx build with 0 errors; Opc.Ua.Server itself builds with 0 warnings.
• Full Opc.Ua.Server.Tests on net10.0: 4707 passed, 0 failed (5 pre-existing skips).
• Targeted: MasterNodeManager/Deterministic/NodeManagement 189/189; NodeManagerLifecycle/MonitoredItemLifecycle/MonitoredItem 406/406; ContinuationPointInBatch (subclass contract) 28/28.
• docs/NodeManagers.md documents the new internal split.

Related Issues

• Fixes [Server] Decouple service-call dispatch from MasterNodeManager lifecycle coordination #4098

Checklist

• I have signed the CLA and read the CONTRIBUTING doc.
• I have added tests that prove my fix is effective or that my feature works and increased code coverage. (pure refactoring; behavior is pinned by the existing MasterNodeManager, deterministic, node-management, lifecycle, and monitored-item suites, which all pass unchanged)
• 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. (full server suite green on net10.0; .NET Framework run pending CI)
• I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings.
• I have addressed all PR feedback received.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Refactors the server-side MasterNodeManager to separate OPC UA session service-call dispatch from NodeManager lifecycle coordination by extracting dispatch logic into a dedicated internal NodeManagerServiceDispatcher, while keeping the public/virtual service surface on MasterNodeManager via new partials. This aligns with the separation requested in #4098 and reduces the size/complexity of the original monolithic class.

Changes:

• Added NodeManagerServiceDispatcher to own routing + per-request validation for Browse/Read/Write/Call/history and monitored-item operations.
• Split MasterNodeManager into partials for service dispatch delegation and node-management (Add/Delete nodes & references) logic.
• Updated docs/NodeManagers.md to document the new internal split.

Reviewed changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Opc.Ua.Server/NodeManager/NodeManagerServiceDispatcher.cs	New internal dispatcher implementing service-call routing and permission validation.
src/Opc.Ua.Server/NodeManager/MasterNodeManager.ServiceDispatch.cs	New partial preserving the public/protected service surface and delegating to the dispatcher.
src/Opc.Ua.Server/NodeManager/MasterNodeManager.NodeManagement.cs	New partial for node-management services with mutation serialization and per-item dispatch.
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs	Updated to initialize and host the dispatcher + retain lifecycle coordination state.
docs/NodeManagers.md	Documents the new internal separation of responsibilities.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Code coverage

✅ Coverage gate passed.

Check	Result	Threshold
✅ Project line rate	86.04% (243663/283185 lines)	>= 70.00%
✅ Project branch rate	75.42%	>= 60.00%
✅ Patch coverage	88.73% (2495/2812 changed lines)	>= 75.00% (> 100 changed lines)
ℹ️ Baseline delta (advisory)	+12.44 pp	73.60% recorded

Uncovered changed lines

• src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs: 1703, 1704, 1754, 1755, 1757, 1758, 1769, 1852, 1853, 1866, 1867, 1868, 1869, 1870, 1871, 1872, 1873, 1874, 1876, 1882, 1883, 1887, 1888, 1889, 1890, 1891, 1892, 1893, 1895, 1896, 1898, 1899, 1900, 1901, 1905, 1906, 1907, 1908, 1909, 1910, 1911, 1913, 1914, 1916, 1917, 1919, 1930, 1948, 1974, 1975, 1979, 1980, 1995, 2097, 2098, 2099, 2143
• src/Opc.Ua.Server/NodeManager/MasterNodeManager.NodeManagement.cs: 92, 94, 97, 98, 100, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 378, 432, 437, 455, 494, 522, 545, 579, 663, 668, 720, 751, 850, 855, 909, 1061, 1076, 1107, 1118
• src/Opc.Ua.Server/NodeManager/NodeManagerServiceDispatcher.cs: 94, 103, 105, 107, 108, 110, 125, 150, 185, 203, 204, 205, 206, 207, 254, 256, 258, 260, 261, 263, 265, 266, 269, 319, 320, 374, 383, 389, 406, 408, 409, 434, 436, 437, 502, 503, 504, 505, 506, 508, 509, 566, 581, 598, 600, 602, 603, 604, 608, 693, 694, 695, 708, 754, 756, 758, 759, 760, 764, 784, 785, 786, 787, 788, 789, 792, 793, 840, 841, 842, 843, 844, 849, 932, 1092, 1097, 1116, 1145, 1310, 1551, 1552, 1582, 1754, 1755, 1810, 1812, 1813, 1871, 1989, 1990, 1996, 1997, 2003, 2004, 2013, 2014, 2015, 2016, 2017, 2018, 2028, 2029, 2076, 2078, 2079, 2096, 2097, 2098, 2116, 2121, 2126, 2127, 2265, 2267, 2268, 2274, 2275, 2276, 2277, 2278, 2279, 2280, 2281, 2282, 2284, 2286, 2287, 2289, 2312, 2317, 2322, 2356, 2357, 2398, 2444, 2445, 2454, 2455, 2456, 2457, 2458, 2459, 2499, 2500, 2501, 2502, 2503, 2504, 2505, 2526, 2530, 2555, 2559, 2563, 2671, 2720, 2725, 2746, 2888, 2893, 3135, 3136, 3156, 3170, 3190, 3200, 3236, 3330, 3334
• src/Opc.Ua.Server/NodeManager/MasterNodeManager.ServiceDispatch.cs: 66, 67, 81, 101, 234, 304, 305, 306, 307, 308, 309, 639, 646, 685, 686, 687, 688, 700, 710, 718, 735, 736, 737, 738, 739, 740, 753, 754, 755, 756, 757, 758, 771, 772, 773, 774, 775, 776, 787, 788, 789, 790, 801, 802, 803, 804, 827, 828, 829, 830, 831, 832, 833, 1007, 1008, 1026, 1027

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

_{Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 85.44%, branch 74.90%.}

[codecov]

Codecov Report

❌ Patch coverage is 86.16071% with 124 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.20%. Comparing base (90a85ed) to head (4ef1457).

Files with missing lines	Patch %	Lines
...r/NodeManager/MasterNodeManager.ServiceDispatch.cs	76.79%	57 Missing and 14 partials ⚠️
...er/NodeManager/MasterNodeManager.NodeManagement.cs	91.01%	34 Missing and 19 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4345      +/-   ##
==========================================
- Coverage   80.21%   80.20%   -0.01%     
==========================================
  Files        2014     2017       +3     
  Lines      275050   275201     +151     
  Branches    47852    47855       +3     
==========================================
+ Hits       220625   220738     +113     
- Misses      37507    37537      +30     
- Partials    16918    16926       +8     

Flag	Coverage Δ
actions	80.20% <86.16%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs	80.16% <ø> (-4.53%)	⬇️
...Server/NodeManager/NodeManagerServiceDispatcher.cs	85.38% <ø> (ø)
...er/NodeManager/MasterNodeManager.NodeManagement.cs	91.01% <91.01%> (ø)
...r/NodeManager/MasterNodeManager.ServiceDispatch.cs	76.79% <76.79%> (ø)

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.