fix(client/dotnet): restore warnings-as-errors floor; license metadata; LazyBrowseNode publication (Client.Dotnet-022..025)

clients/dotnet/Directory.Build.props

@@ -1,4 +1,17 @@
 <Project>
+  <PropertyGroup>
+    <!-- Build-quality enforcement floor, mirroring src/Directory.Build.props so the
+         .NET client tree is held to the same baseline CLAUDE.md mandates (warnings as
+         errors, code-style enforced at build, latest analyzers, deterministic builds). -->
+    <LangVersion>latest</LangVersion>
+    <Nullable>enable</Nullable>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
+    <AnalysisLevel>latest</AnalysisLevel>
+    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
+    <Deterministic>true</Deterministic>
+  </PropertyGroup>
+
   <PropertyGroup>
     <!-- Shared package metadata for clients/dotnet/. Individual projects opt in via <IsPackable>true</IsPackable>. -->
     <Authors>Joseph Doherty</Authors>
@@ -10,11 +23,15 @@
     <PackageProjectUrl>https://gitea.dohertylan.com/dohertj2/mxaccessgw</PackageProjectUrl>
     <PackageTags>mxaccess;mxgateway;grpc;client;archestra</PackageTags>
     <PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
+    <!-- Proprietary/internal package, consistent with the Rust ("Proprietary") and
+         Python ("Proprietary") client license declarations. A LicenseRef SPDX expression
+         is rejected by the current NuGet toolset (NU5124), so the proprietary terms ship
+         as a packaged license file instead. -->
+    <PackageLicenseFile>LICENSE.txt</PackageLicenseFile>
     <!-- Versioning: bump per release. Symbols ship as snupkg. -->
     <Version>0.1.0</Version>
     <IncludeSymbols>true</IncludeSymbols>
     <SymbolPackageFormat>snupkg</SymbolPackageFormat>
-    <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <!-- Default: do NOT pack. Each project opts in. -->
     <IsPackable>false</IsPackable>
   </PropertyGroup>

clients/dotnet/LICENSE.txt

@@ -0,0 +1,12 @@
+Proprietary License
+
+Copyright (c) ZB MOM WW. All rights reserved.
+
+This software and its source code are proprietary and confidential. They are
+licensed, not sold, for internal use within ZB MOM WW and its authorized
+partners only. No part of this package may be reproduced, distributed, or
+transmitted to third parties without the prior written permission of ZB MOM WW.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE, AND NONINFRINGEMENT.

clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/FakeGalaxyRepositoryTransport.cs

@@ -135,25 +135,35 @@ internal sealed class FakeGalaxyRepositoryTransport(MxGatewayClientOptions optio
     /// <summary>Queue of exceptions to throw from BrowseChildren; dequeued in FIFO order.</summary>
     public Queue<Exception> BrowseChildrenExceptions { get; } = new();
 
+    /// <summary>
+    /// Optional hook awaited inside BrowseChildren before the reply is produced. Lets a
+    /// test hold an RPC mid-flight to exercise concurrent reads of the in-progress node.
+    /// </summary>
+    public Func<Task>? BrowseChildrenGate { get; set; }
+
     /// <summary>
     /// Records the request and either throws a queued exception or returns the configured reply.
     /// </summary>
     /// <param name="request">The BrowseChildrenRequest to process.</param>
     /// <param name="callOptions">Call options specifying RPC behavior.</param>
-    public Task<BrowseChildrenReply> BrowseChildrenAsync(
+    public async Task<BrowseChildrenReply> BrowseChildrenAsync(
         BrowseChildrenRequest request,
         CallOptions callOptions)
     {
         BrowseChildrenCalls.Add((request, callOptions));
         if (BrowseChildrenExceptions.TryDequeue(out Exception? exception))
         {
-            return Task.FromException<BrowseChildrenReply>(exception);
+            throw exception;
         }
 
-        return Task.FromResult(
+        if (BrowseChildrenGate is { } gate)
-            BrowseChildrenReplies.TryDequeue(out BrowseChildrenReply? reply)
+        {
-                ? reply
+            await gate().ConfigureAwait(false);
-                : BrowseChildrenReply);
+        }
+
+        return BrowseChildrenReplies.TryDequeue(out BrowseChildrenReply? reply)
+            ? reply
+            : BrowseChildrenReply;
     }
 
     /// <summary>

clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/LazyBrowseNodeTests.cs

@@ -175,6 +175,96 @@ public sealed class LazyBrowseNodeTests
         Assert.Equal(2, transport.BrowseChildrenCalls.Count);
     }
 
+    /// <summary>
+    /// Verifies that reading Children/IsExpanded concurrently with an in-flight ExpandAsync
+    /// never throws (no torn enumeration of a mid-append list) and, once IsExpanded flips to
+    /// true, the published Children snapshot is fully populated. Pins the safe-publication
+    /// contract on the lock-free readers (Client.Dotnet-025).
+    /// </summary>
+    [Fact]
+    public async Task Expand_ConcurrentReadOfChildren_NeverTearsAndPublishesAtomically()
+    {
+        FakeGalaxyRepositoryTransport transport = CreateTransport();
+        transport.BrowseChildrenReplies.Enqueue(BuildReply(
+            children: [BuildObject(1, "Plant", isArea: true)],
+            childHasChildren: [true],
+            cacheSequence: 1));
+
+        // Multi-page child set so the expand loop spends meaningful time appending,
+        // widening the window for a concurrent reader to observe a torn list.
+        BrowseChildrenReply childPage1 = BuildReply(
+            children: [BuildObject(10, "A"), BuildObject(11, "B"), BuildObject(12, "C")],
+            childHasChildren: [false, false, false],
+            cacheSequence: 1);
+        childPage1.NextPageToken = "1:p:3";
+        transport.BrowseChildrenReplies.Enqueue(childPage1);
+        transport.BrowseChildrenReplies.Enqueue(BuildReply(
+            children: [BuildObject(13, "D"), BuildObject(14, "E")],
+            childHasChildren: [false, false],
+            cacheSequence: 1));
+
+        await using GalaxyRepositoryClient client = CreateClient(transport);
+        IReadOnlyList<LazyBrowseNode> roots = await client.BrowseAsync();
+        LazyBrowseNode node = roots[0];
+
+        // Gate the child-page RPCs so the expand stays mid-flight while the reader spins.
+        using SemaphoreSlim release = new(0, 1);
+        bool firstChildCall = true;
+        transport.BrowseChildrenGate = async () =>
+        {
+            if (firstChildCall)
+            {
+                firstChildCall = false;
+                await release.WaitAsync().ConfigureAwait(false);
+            }
+        };
+
+        using CancellationTokenSource readerStop = new();
+        Exception? readerFailure = null;
+        Task reader = Task.Run(() =>
+        {
+            try
+            {
+                while (!readerStop.IsCancellationRequested)
+                {
+                    bool expanded = node.IsExpanded;
+
+                    // Enumerate the snapshot; a torn/mid-append list would throw here.
+                    int count = 0;
+                    foreach (LazyBrowseNode _ in node.Children)
+                    {
+                        count++;
+                    }
+
+                    // If the node reports expanded, the published snapshot must be complete.
+                    if (expanded)
+                    {
+                        Assert.Equal(5, count);
+                    }
+                }
+            }
+            catch (Exception ex)
+            {
+                readerFailure = ex;
+            }
+        });
+
+        Task expand = node.ExpandAsync();
+        // Let the reader spin against the empty pre-publication snapshot for a moment.
+        await Task.Delay(50);
+        release.Release();
+        await expand;
+
+        // Let the reader observe the post-publication state, then stop it.
+        await Task.Delay(50);
+        readerStop.Cancel();
+        await reader;
+
+        Assert.Null(readerFailure);
+        Assert.True(node.IsExpanded);
+        Assert.Equal(5, node.Children.Count);
+    }
+
     /// <summary>
     /// Verifies that BrowseChildrenOptions filter fields are forwarded to the BrowseChildren request.
     /// </summary>

clients/dotnet/ZB.MOM.WW.MxGateway.Client/LazyBrowseNode.cs

@@ -12,9 +12,14 @@ public sealed class LazyBrowseNode
 {
     private readonly GalaxyRepositoryClient _client;
     private readonly BrowseChildrenOptions _options;
-    private readonly List<LazyBrowseNode> _children = [];
     private readonly SemaphoreSlim _expandLock = new(1, 1);
-    private bool _isExpanded;
+
+    // Published once, under _expandLock, when expansion completes. Lock-free readers
+    // see either the empty pre-expansion snapshot or the fully-populated post-expansion
+    // snapshot — never a partially-filled list — because the snapshot is built in a local
+    // and handed off via Volatile.Write (release) paired with Volatile.Read (acquire).
+    private IReadOnlyList<LazyBrowseNode> _children = [];
+    private volatile bool _isExpanded;
 
     internal LazyBrowseNode(
         GalaxyRepositoryClient client,
@@ -35,7 +40,7 @@ public sealed class LazyBrowseNode
     public bool HasChildrenHint { get; }
 
     /// <summary>Direct children loaded by <see cref="ExpandAsync"/>; empty until then.</summary>
-    public IReadOnlyList<LazyBrowseNode> Children => _children;
+    public IReadOnlyList<LazyBrowseNode> Children => Volatile.Read(ref _children);
 
     /// <summary>True after the first <see cref="ExpandAsync"/> call completes.</summary>
     public bool IsExpanded => _isExpanded;
@@ -46,7 +51,13 @@ public sealed class LazyBrowseNode
     /// </summary>
     /// <remarks>
     ///     Thread-safe: concurrent callers see exactly one fetch; subsequent callers
-    ///     (after the first completes) return immediately.
+    ///     (after the first completes) return immediately. <see cref="Children"/> and
+    ///     <see cref="IsExpanded"/> may be read concurrently with an in-flight
+    ///     <see cref="ExpandAsync"/> on another thread; the populated children are
+    ///     published as an immutable snapshot under a release barrier, so a reader that
+    ///     observes <see cref="IsExpanded"/> as <see langword="true"/> always sees the
+    ///     fully-populated <see cref="Children"/>, and a reader never enumerates a
+    ///     partially-built list.
     /// </remarks>
     /// <param name="cancellationToken">Token to observe for cancellation.</param>
     public async Task ExpandAsync(CancellationToken cancellationToken = default)
@@ -64,6 +75,10 @@ public sealed class LazyBrowseNode
                 return;
             }
 
+            // Accumulate into a local list, never the published field, so a lock-free
+            // reader can never observe a half-populated collection or enumerate a list
+            // that is being mutated mid-append.
+            List<LazyBrowseNode> children = [];
             string pageToken = string.Empty;
             HashSet<string> seenPageTokens = new(StringComparer.Ordinal);
             do
@@ -79,7 +94,7 @@ public sealed class LazyBrowseNode
                 for (int i = 0; i < reply.Children.Count; i++)
                 {
                     bool hint = i < reply.ChildHasChildren.Count && reply.ChildHasChildren[i];
-                    _children.Add(new LazyBrowseNode(_client, reply.Children[i], hint, _options));
+                    children.Add(new LazyBrowseNode(_client, reply.Children[i], hint, _options));
                 }
 
                 pageToken = reply.NextPageToken;
@@ -91,6 +106,10 @@ public sealed class LazyBrowseNode
             }
             while (!string.IsNullOrWhiteSpace(pageToken));
 
+            // Publish the completed, immutable snapshot (release) before marking the node
+            // expanded (the volatile write below). A reader that observes IsExpanded == true
+            // is guaranteed to also observe the fully-populated Children.
+            Volatile.Write(ref _children, children);
             _isExpanded = true;
         }
         finally

clients/dotnet/ZB.MOM.WW.MxGateway.Client/ZB.MOM.WW.MxGateway.Client.csproj

@@ -21,10 +21,15 @@
     <PackageId>ZB.MOM.WW.MxGateway.Client</PackageId>
     <Description>.NET 10 gRPC client for the MxAccessGateway service. Provides typed wrappers, retry, and a lazy-browse walker over the Galaxy Repository hierarchy.</Description>
     <PackageReadmeFile>README.md</PackageReadmeFile>
+    <!-- Only the shipped library generates XML docs (matching src/Contracts). The Cli and
+         Tests projects are not packable and do not document their public surface, so this
+         stays out of the shared Directory.Build.props to avoid CS1591 on test classes. -->
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
   </PropertyGroup>
 
   <ItemGroup>
     <None Include="..\README.md" Pack="true" PackagePath="\" />
+    <None Include="..\LICENSE.txt" Pack="true" PackagePath="\" />
   </ItemGroup>
 
   <ItemGroup>

code-reviews/Client.Dotnet/findings.md

@@ -4,8 +4,8 @@
 |---|---|
 | Module | `clients/dotnet` |
 | Reviewer | Claude Code |
-| Review date | 2026-05-24 |
+| Review date | 2026-06-15 |
-| Commit reviewed | `42b0037` |
+| Commit reviewed | `410acc9` |
 | Status | Re-reviewed |
 | Open findings | 0 |
 
@@ -383,6 +383,40 @@ Re-review pass at `42b0037`. Diff against `d692232` consists of four commits:
 | 9 | Testing coverage | No new issues — `RunAsync_StreamAlarms_*`, `RunAsync_AcknowledgeAlarm_*`, and `RunAsync_Batch_*` give the new surface unit coverage. `bench-read-bulk` is the same stress-harness-not-SDK shape called out in the prior re-review and is not flagged here. |
 | 10 | Documentation & comments | Issue found (this review): the README examples for the two new alarm CLI subcommands cite wrong flag names and a non-existent `--session-id` (Client.Dotnet-018). The new XML docs on `StreamAlarmsAsync` / `AcknowledgeAlarmAsync` and on the bulk SDK methods are accurate and complete. |
 
+#### 2026-06-15 re-review (commit 410acc9)
+
+Re-review pass at `410acc9`. The diff against `42b0037` is packaging/release metadata
+(NuGet/Gitea feed), a TLS trust-posture option (`RequireCertificateValidation` + a
+lenient accept-all default for the gateway's auto-generated self-signed cert), the
+Galaxy `BrowseChildren` RPC plumbing plus a `LazyBrowseNode` lazy-browse walker, and
+in-source resolutions of the prior pass's Client.Dotnet-018..021 (CLI flag-name README
+fix, `RequireRegisterServerHandle`, `ParseTimeoutMs` negative guard, steady-state OCE
+filter). The alarm-provider-fallback proto surface mentioned in the review brief is
+**not** present in this diff — no `AlarmProviderMode` / `AlarmProviderStatus` /
+`source_provider` / provider-mode-changed event reaches the .NET client here.
+
+Build is green (`dotnet build … .slnx` succeeds) and all 78 unit tests pass (1 skipped
+live smoke). The build now emits **10 CS1591 warnings** that do not break the build,
+because the `clients/dotnet/Directory.Build.props` enforcement floor recorded as
+resolved under Client.Dotnet-012 (`TreatWarningsAsErrors` / `EnforceCodeStyleInBuild` /
+`AnalysisLevel` / `Deterministic`) is **absent** from the history that reaches HEAD —
+the props file at HEAD is packaging-metadata-only (Client.Dotnet-022). `git merge-base
+--is-ancestor a020350 HEAD` is false: the 2026-05-20 review-sweep commit that resolved
+012 is not in this line of history.
+
+| # | Category | Result |
+|---|---|---|
+| 1 | Correctness & logic bugs | No new issues. The Galaxy `BrowseAsync` / `LazyBrowseNode.ExpandAsync` pagination correctly drains `next_page_token`, re-binds the same parent selector + filter set per page (matching the opaque-token contract), and guards against repeated tokens; the per-child `child_has_children` hint is read with an index-bounds check. The Client.Dotnet-019/021 in-source fixes (`RequireRegisterServerHandle`, `ParseTimeoutMs`) are correctly applied. |
+| 2 | mxaccessgw conventions | Issue found (this review): the `clients/dotnet/Directory.Build.props` enforcement floor (warnings-as-errors / code-style enforcement) mandated by CLAUDE.md and recorded resolved under Client.Dotnet-012 is missing at HEAD; the new props file carries only packaging metadata (Client.Dotnet-022). Consumes the shared contracts project, no forked proto, `authorization: Bearer` metadata correct. |
+| 3 | Concurrency & thread safety | Issue found (this review): `LazyBrowseNode.Children` and `IsExpanded` are read lock-free while `ExpandAsync` mutates `_children` and writes `_isExpanded` under `_expandLock`, with no release/acquire barrier to a concurrent reader (Client.Dotnet-025). `ExpandAsync`'s one-RPC dedup itself is correct (double-checked under the lock). |
+| 4 | Error handling & resilience | No new issues — `BrowseChildrenAsync` routes `RpcException` through the shared `MapRpcException`; the bench steady-state OCE filter (Client.Dotnet-020) is correctly applied. |
+| 5 | Security | No committed secret — the README Gitea-feed `dotnet nuget add source` example uses `<gitea-username>` / `<gitea-token-or-password>` placeholders. Note: TLS is lenient-by-default (accept-all callback when `UseTls` and no pinned CA), which disables certificate verification / MITM protection; this is an explicit, documented design choice for the gateway's auto-generated self-signed cert and is opt-out via `RequireCertificateValidation` or CA pinning, so not flagged as a finding. |
+| 6 | Performance & resource management | No issues found — `LazyBrowseNode` holds one `SemaphoreSlim` per node (never disposed, but it owns no unmanaged handle and the node lifetime is the tree's); browse paging caps at 500/page. |
+| 7 | Design-document adherence | No issues found — `BrowseChildren` / lazy-browse match `docs/GalaxyRepository.md#browsechildren`; the TLS posture matches `docs/GatewayConfiguration.md` (`RequireCertificateValidation` default `false`) and `DotnetClientDesign.md`. |
+| 8 | Code organization & conventions | Issue found (this review): Client.Dotnet-022 (lost enforcement props); the new `GenerateDocumentationFile=true` in the shared props also applies to the Cli and Tests projects, surfacing CS1591 on `IMxGatewayCliClient` and every test class (Client.Dotnet-023); the client (and Contracts) NuGet package ships with no `<license>` metadata despite setting `PackageRequireLicenseAcceptance=false` (Client.Dotnet-024). The nuspec correctly emits the transitive `ZB.MOM.WW.MxGateway.Contracts 0.1.0` dependency, so the README "pulled in transitively" claim holds. |
+| 9 | Testing coverage | No new issues — `LazyBrowseNodeTests` (7 cases incl. multi-page, concurrent-expand-one-RPC, filter forwarding), `MxGatewayClientTlsHandlerTests` / `GalaxyRepositoryClientTlsHandlerTests`, and the README-example parse tests give the new surface good coverage. |
+| 10 | Documentation & comments | No new issues — README NuGet-install / lazy-browse / TLS-trust sections are accurate, cross-doc anchors (`#automatic-self-signed-certificate`, `#browsechildren`) resolve, and the new XML docs on `BrowseAsync` / `LazyBrowseNode` / `RequireCertificateValidation` are complete. (The CS1591-surfaced missing docs are tracked under Client.Dotnet-023.) |
+
 ### Client.Dotnet-018
 
 | Field | Value |
@@ -507,3 +541,65 @@ uint timeoutMs = (uint)timeoutMsRaw;
 A single shared helper (e.g. `ParseTimeoutMs(CliArguments, string, int)`) on `MxGatewayClientCli` would cover both call sites and remove the duplication.
 
 **Resolution:** 2026-05-24 — Confirmed against source: both `ReadBulkAsync` (line 490) and `BenchReadBulkAsync` (line 715) cast `arguments.GetInt32("timeout-ms", ...)` straight to `uint`, so `--timeout-ms -1` silently wrapped to `0xFFFFFFFF` (~49.7 days). Added a single shared private helper `ParseTimeoutMs(CliArguments arguments, int defaultValue)` on `MxGatewayClientCli` that reads the int32, rejects negatives with a clear `ArgumentException` ("--timeout-ms must be a non-negative integer (use 0 for the gateway default)."), and returns the safe `(uint)`. Both call sites now route through the helper. Regression test `MxGatewayClientCliTests.RunAsync_TimeoutMs_NegativeValue_RejectsWithClearError` (xUnit `[Theory]` over `read-bulk` and `bench-read-bulk`) drives the CLI with `--timeout-ms -1` and asserts the exit code is non-zero, that stderr contains "timeout-ms", and that the "non-negative" guard text is present. Verified red against the original `(uint)arguments.GetInt32(...)` casts (the bench proceeded past the timeout parse and tripped a downstream "Queue empty" error rather than the descriptive guard message) and green after the helper landed.
+
+### Client.Dotnet-022
+
+| Field | Value |
+|---|---|
+| Severity | Medium |
+| Category | mxaccessgw conventions |
+| Location | `clients/dotnet/Directory.Build.props:1-21` |
+| Status | Resolved |
+
+**Description:** Client.Dotnet-012 was recorded resolved (2026-05-20, commit `a020350`) by adding `clients/dotnet/Directory.Build.props` mirroring `src/Directory.Build.props` — `TreatWarningsAsErrors=true`, `EnforceCodeStyleInBuild=true`, `AnalysisLevel=latest`, `Deterministic=true`, `LangVersion=latest`, `Nullable=enable`, `ImplicitUsings=enable` — to restore the build-quality floor that `CLAUDE.md` calls a baseline for the .NET client. That enforcement props file is **not present in the line of history that reaches HEAD**: `git merge-base --is-ancestor a020350 HEAD` is false (the 2026-05-20 review-sweep commit was dropped during the `ZB.MOM.WW` rename / history rebuild). At `42b0037` the file did not exist at all (`git show 42b0037:clients/dotnet/Directory.Build.props` fails), and at HEAD commit `523f944` introduced a **new** `clients/dotnet/Directory.Build.props` that carries only NuGet packaging metadata (Authors/Company/RepositoryUrl/Version/etc.) — none of the enforcement properties. None of the three client `.csproj` files set `TreatWarningsAsErrors` or `EnforceCodeStyleInBuild` independently (they set only `TargetFramework` and `Nullable`).
+
+Net effect at HEAD: `dotnet build clients/dotnet/ZB.MOM.WW.MxGateway.Client.slnx` **succeeds with 10 CS1591 warnings** instead of failing. The mandated quality gate that would turn new warnings (missing docs, analyzer findings, code-style violations) into build breaks is gone for the entire client tree. This is a regression of the previously-closed Client.Dotnet-012; recorded as a fresh finding at the new commit per the re-review process.
+
+**Recommendation:** Restore the enforcement properties in `clients/dotnet/Directory.Build.props` alongside the packaging metadata (they can coexist in the same `<Project>`), or add a sibling `clients/dotnet/Directory.Build.props` import. Re-run `dotnet build …slnx` and confirm 0 warnings / 0 errors (which will require closing Client.Dotnet-023 too, since the CS1591 warnings would otherwise become errors). Add a guard so the floor is not silently dropped again — e.g. assert the property is set in a small build test or CI check.
+
+**Resolution:** 2026-06-15 — Confirmed at HEAD: `clients/dotnet/Directory.Build.props` carried only packaging metadata; none of the three client `.csproj` files set the enforcement properties, so `dotnet build …slnx` succeeded with 10 CS1591 warnings instead of failing. Restored the enforcement floor in `clients/dotnet/Directory.Build.props` mirroring `src/Directory.Build.props` (`LangVersion=latest`, `Nullable=enable`, `ImplicitUsings=enable`, `TreatWarningsAsErrors=true`, `AnalysisLevel=latest`, `EnforceCodeStyleInBuild=true`, `Deterministic=true`) in a second `<PropertyGroup>` alongside the existing packaging metadata. Resolved jointly with Client.Dotnet-023 (the CS1591 warnings would otherwise become errors under the restored `TreatWarningsAsErrors`). `dotnet build clients/dotnet/ZB.MOM.WW.MxGateway.Client.slnx -t:Rebuild` now reports 0 Warning(s) / 0 Error(s).
+
+### Client.Dotnet-023
+
+| Field | Value |
+|---|---|
+| Severity | Low |
+| Category | Code organization & conventions |
+| Location | `clients/dotnet/Directory.Build.props:17`, `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/IMxGatewayCliClient.cs:6`, `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/*.cs` |
+| Status | Resolved |
+
+**Description:** The new shared `clients/dotnet/Directory.Build.props` sets `GenerateDocumentationFile=true` at the directory level, so it applies to all three projects — including `ZB.MOM.WW.MxGateway.Client.Cli` and `ZB.MOM.WW.MxGateway.Client.Tests`, which are not packable and were not previously generating an XML doc file. Turning it on surfaces 10 CS1591 "missing XML comment" warnings: `IMxGatewayCliClient` (the public CLI interface, never documented at the type level — note Client.Dotnet-013's resolution claimed a type-level summary was added, but it is absent in the history reaching HEAD for the same reason as Client.Dotnet-022) plus every public xUnit test class (`GalaxyRepositoryClientTests`, `MxGatewayClientTlsHandlerTests`, `GalaxyRepositoryClientTlsHandlerTests`, and seven others). Today these are only warnings because the enforcement floor is missing (Client.Dotnet-022); once that floor is restored they become build-breaking errors.
+
+**Recommendation:** Scope `GenerateDocumentationFile=true` to the packable library project only (move it from the shared props into `ZB.MOM.WW.MxGateway.Client.csproj`, which is the only project that ships a `.nupkg`), or keep it directory-wide but suppress CS1591 on the non-public test/CLI assemblies (`<NoWarn>$(NoWarn);CS1591</NoWarn>` in those two `.csproj` files) and add the one-line type summary to `IMxGatewayCliClient`. The first option is cleaner and avoids documenting test classes.
+
+**Resolution:** 2026-06-15 — Confirmed via `-t:Rebuild`: the directory-wide `GenerateDocumentationFile=true` surfaced exactly 10 CS1591 warnings — `IMxGatewayCliClient` plus nine xUnit test classes (`GalaxyRepositoryClientTests`, `MxCommandReplyExtensionsTests`, `MxGatewayClientContractInfoTests`, `MxGatewayClientOptionsTests`, `MxGatewayClientTlsHandlerTests`, `GalaxyRepositoryClientTlsHandlerTests`, `MxGatewayGeneratedContractTests`, `MxStatusProxyExtensionsTests`, `MxValueExtensionsTests`); the shipped Client library itself emitted zero (its public surface was already fully documented). Took the first (cleaner) option, matching how `src/` handles this — only the packable `src/ZB.MOM.WW.MxGateway.Contracts.csproj` sets `GenerateDocumentationFile` directly. Removed `GenerateDocumentationFile=true` from the shared `clients/dotnet/Directory.Build.props` and moved it into the packable `ZB.MOM.WW.MxGateway.Client.csproj` only, so the Cli and Tests projects no longer generate doc files and CS1591 is not raised against them. No doc comments were added to test classes. With the Client.Dotnet-022 floor restored, the rebuild is clean (0 warnings / 0 errors).
+
+### Client.Dotnet-024
+
+| Field | Value |
+|---|---|
+| Severity | Low |
+| Category | Code organization & conventions |
+| Location | `clients/dotnet/Directory.Build.props:12`, `clients/dotnet/ZB.MOM.WW.MxGateway.Client/ZB.MOM.WW.MxGateway.Client.csproj:19-24` |
+| Status | Resolved |
+
+**Description:** The client package sets `PackageRequireLicenseAcceptance=false` but declares **no license at all** — there is no `PackageLicenseExpression` and no `PackageLicenseFile` in `clients/dotnet/Directory.Build.props` or in the packable `.csproj`. Confirmed by packing: the emitted `ZB.MOM.WW.MxGateway.Client.0.1.0.nuspec` has no `<license>` element, so the produced package carries no license metadata and a NuGet feed renders it as "License: not specified." The sibling `ZB.MOM.WW.MxGateway.Contracts` package (the transitive dependency) has the same gap. `dotnet pack` does not warn (a missing license is allowed), so the omission is silent. Setting `PackageRequireLicenseAcceptance=false` while shipping no license is internally inconsistent — that flag exists to control acceptance of a license that should be present.
+
+**Recommendation:** Add the intended license to `clients/dotnet/Directory.Build.props` (and to `ZB.MOM.WW.MxGateway.Contracts.csproj` for parity) — either `<PackageLicenseExpression>` with an SPDX id (e.g. a proprietary marker or the actual license) or `<PackageLicenseFile>` pointing at a committed `LICENSE`. If the package is intentionally unlicensed/internal-only, document that explicitly rather than leaving the field blank.
+
+**Resolution:** 2026-06-15 — Confirmed via pack: the emitted nuspec had no `<license>` element. Marked the package "Proprietary" consistent with the other clients' decision (Rust `license = "Proprietary"`, Python `license = { text = "Proprietary" }` + `License :: Other/Proprietary License`). A `<PackageLicenseExpression>LicenseRef-Proprietary</PackageLicenseExpression>` was tried first but the current NuGet toolset rejects `LicenseRef-*` (NU5124), which the restored `TreatWarningsAsErrors` escalates to a pack failure — so the proprietary terms ship as a committed license file instead: added `clients/dotnet/LICENSE.txt` (proprietary/internal-use terms), set `<PackageLicenseFile>LICENSE.txt</PackageLicenseFile>` in the shared `clients/dotnet/Directory.Build.props`, and packed it at the package root via a `<None Include="..\LICENSE.txt" Pack="true" PackagePath="\" />` item in the packable `ZB.MOM.WW.MxGateway.Client.csproj`. `dotnet pack` now succeeds and the nuspec carries `<license type="file">LICENSE.txt</license>` with `LICENSE.txt` present in the `.nupkg`. Scope was limited to Client.Dotnet per the constraints — the sibling `ZB.MOM.WW.MxGateway.Contracts` package has the same gap and is NOT touched here (it is a different module; flagging it for that module's review).
+
+### Client.Dotnet-025
+
+| Field | Value |
+|---|---|
+| Severity | Low |
+| Category | Concurrency & thread safety |
+| Location | `clients/dotnet/ZB.MOM.WW.MxGateway.Client/LazyBrowseNode.cs:38,41,54,82,94` |
+| Status | Resolved |
+
+**Description:** `LazyBrowseNode.ExpandAsync` is explicitly documented as thread-safe ("concurrent callers see exactly one fetch"), and its one-RPC dedup is correct: it double-checks `_isExpanded` under `_expandLock`. But the *readers* of the results are lock-free. `Children => _children` returns the live backing `List<LazyBrowseNode>` reference, and `IsExpanded => _isExpanded` reads the plain `bool` field — neither takes `_expandLock` nor uses `Volatile`. A thread that observes `IsExpanded == true` (or simply enumerates `Children`) concurrently with the writer thread inside `ExpandAsync` has no release/acquire barrier guaranteeing it sees the fully-populated `_children` contents that were appended under the lock. On x86/x64 the bool read and the list-reference read are atomic and the practical risk is low, but the published-state visibility is not guaranteed by the memory model, and a reader enumerating `Children` while a concurrent `ExpandAsync` is mid-append can throw `InvalidOperationException` ("collection was modified"). This is inconsistent with the type's own thread-safety claim.
+
+**Recommendation:** Either (a) tighten the documented contract to "ExpandAsync is safe to call concurrently, but Children/IsExpanded must only be read after the awaited ExpandAsync completes (no concurrent reader/expander)", or (b) make the publication safe: write `_isExpanded` via `Volatile.Write` and read via `Volatile.Read`, and return an immutable snapshot from `Children` (e.g. assign a completed `IReadOnlyList` under the lock and expose that field) so lock-free readers never observe a partially-populated list. Option (a) is the smallest change and matches the realistic usage (UI thread expands then renders).
+
+**Resolution:** 2026-06-15 — Confirmed against source: `Children => _children` returned the live mutable backing `List<LazyBrowseNode>` and `IsExpanded => _isExpanded` read a plain `bool`, while `ExpandAsync` appended to that same list under `_expandLock` with no release/acquire barrier to lock-free readers — so a concurrent reader could enumerate a mid-append list and throw `InvalidOperationException` ("collection was modified"). Applied option (b) (safe publication): `ExpandAsync` now accumulates children into a method-local `List<LazyBrowseNode>` and, only when fully drained across all pages, publishes it via `Volatile.Write(ref _children, children)` (release) immediately before setting the now-`volatile bool _isExpanded = true`. The `_children` field is an `IReadOnlyList<LazyBrowseNode>` read via `Volatile.Read` from the `Children` getter (acquire), so a reader that observes `IsExpanded == true` always sees the fully-populated snapshot and never enumerates a partially-built list. Updated the `ExpandAsync` `<remarks>` to document the strengthened concurrent-read guarantee. Regression test `LazyBrowseNodeTests.Expand_ConcurrentReadOfChildren_NeverTearsAndPublishesAtomically` gates the child-page RPCs (via a new `FakeGalaxyRepositoryTransport.BrowseChildrenGate` hook) to hold the expand mid-flight while a background reader spins enumerating `Children` and reading `IsExpanded`, asserting no exception escapes and that once `IsExpanded` is true the published snapshot has all five children. Verified red against the pre-fix code (the reader threw `InvalidOperationException: Collection was modified` deterministically across three runs) and green after the fix.