From 5d4dd17d7f2513a35c6e676a5359ac69782b53f5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 07:58:19 -0400 Subject: [PATCH 1/4] fix(mqtt-transport): warn when a configured password is dropped for want of a username MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driver.Mqtt.Transport-002 (Low, correctness). Credentials were attached only when Username was non-empty, so a config carrying a Password and no Username connected anonymously with the secret silently discarded — the deployment reads as authenticated and is not, which is the direction an operator cannot see from the config. Warned rather than sent. MQTT 3.1.1 may not carry a password without a username, so the password-only CONNECT is not available on every protocol version this factory serves, and synthesizing one under V500 only would change the wire behaviour of deployments that connect happily today. The configured values stay authoritative; the drop just stops being silent. The secret is never formatted into the message. Tests: the warning fires and does NOT contain the password, plus the falsifying control that an ordinary anonymous config (neither credential set) stays silent — without it the fix could warn on every connect and still pass. --- .../MqttClientOptionsFactory.cs | 12 ++++++ .../MqttConnectionTests.cs | 37 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs index 0f7e4333..17dde107 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs @@ -98,6 +98,18 @@ public static class MqttClientOptionsFactory { builder = builder.WithCredentials(options.Username, options.Password ?? string.Empty); } + else if (!string.IsNullOrEmpty(options.Password)) + { + // A password with no username is dropped, and the drop is the direction an operator cannot + // see: the config reads as authenticated and the CONNECT goes out anonymous. MQTT 3.1.1 may + // not carry a password without a username, so synthesizing one is not available on every + // protocol version this factory serves — and inventing a password-only CONNECT under 5.0 + // only would change the wire behaviour of deployments that connect happily today. Say so + // once, here, and leave the configured values authoritative. Never format the secret. + logger?.LogWarning( + "MQTT: a password is configured with no username, so it is NOT being sent — this connection " + + "will be made anonymously. Set the username, or clear the password to make that intentional."); + } builder = builder.WithTlsOptions(tls => ConfigureTls(tls, options, logger)); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs index f6a82601..315ca370 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttConnectionTests.cs @@ -366,6 +366,43 @@ public sealed class MqttConnectionTests built.Credentials.GetUserName(built).ShouldBe("svc"); } + /// + /// Driver.Mqtt.Transport-002. A configured password with no username is dropped — MQTT + /// 3.1.1 may not carry one without the other, and this factory does not synthesize the + /// password-only CONNECT 5.0 would permit. What must not happen is the drop being silent: the + /// deployment then believes it authenticates and does not, which is the one direction an + /// operator cannot see from the config. The secret itself stays out of the log. + /// + [Fact] + public void BuildClientOptions_PasswordWithoutUsername_IsDroppedLoudlyAndWithoutLeakingIt() + { + const string secret = "s3cr3t-that-must-not-be-logged"; + var log = new CapturingLogger(); + + var built = MqttClientOptionsFactory.BuildClientOptions( + new MqttDriverOptions { Host = Host, Password = secret }, clientIdSuffix: null, logger: log); + + built.Credentials.ShouldBeNull(); + log.CountContaining("password").ShouldBe(1); + log.CountContaining(secret).ShouldBe(0); + } + + /// + /// The falsifying control for the test above: an ordinary anonymous deployment configures + /// neither credential and must stay silent, or every such driver warns on every connect. + /// + [Fact] + public void BuildClientOptions_NoCredentialsAtAll_SaysNothing() + { + var log = new CapturingLogger(); + + MqttClientOptionsFactory.BuildClientOptions( + new MqttDriverOptions { Host = Host }, clientIdSuffix: null, logger: log) + .Credentials.ShouldBeNull(); + + log.CountContaining("password").ShouldBe(0); + } + /// The ctor must not touch the network — the universal-browser throwaway-instance pattern. [Fact] public void Ctor_IsConnectionFree() From 2298485bdadc19c7a622c0f886ccd55312e3ae94 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 07:58:29 -0400 Subject: [PATCH 2/4] fix(mqtt): a reconnect that re-establishes nothing tears the session down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driver.Mqtt-007 (Low, error handling). OnReconnectedAsync throws when a re-subscribe is granted zero filters — MqttConnection's contract is that a throwing Reconnected subscriber tears the fresh session down and retries under backoff, because a connected-but-deaf driver is the one outcome this path must never serve. But when the live subscribed topics resolved to zero filters in the first place it logged a warning and returned, reaching the same "nothing was re-established" state by a different route and giving it the opposite answer. Throw there too, and say so in the remarks so the type states ONE rule. It costs no other tag — every live topic is in the set that resolved to nothing, so there is nothing left flowing to protect, which is the whole reason the partial- rejection case is exempt — and it is not permanent: the retry runs against whatever Register has published by then. Test reaches the state the only way production can: Register prunes _subscribedTopics of the topics its new table no longer carries, but a SUBSCRIBE already in flight records its own topic AFTER that prune ran, so a deploy landing inside the round-trip leaves a live topic no authored tag resolves to. Paired with the control that a reconnect with no live subscriptions at all stays a no-op — throwing there would tear down the session of every driver whose tags have not been subscribed yet. --- .../MqttSubscriptionManager.cs | 34 +++++++----- .../MqttSubscriptionManagerTests.cs | 53 +++++++++++++++++++ 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs index 14dc9d1d..320a6a04 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs @@ -397,6 +397,15 @@ public sealed class MqttSubscriptionManager /// rest keep flowing. That is the plan's "SUBACK failure → per-ref Bad, not hang", scoped /// to the case where it does not cost the whole driver. /// + /// + /// Live topics resolving to zero filters is a total failure too — it re-establishes + /// nothing, so it throws by the same rule rather than warning and returning. It costs no + /// other tag (every live topic is in the set that resolved to nothing) and it is not + /// permanent, because the retry runs against whatever has published + /// by then. It is reachable only when a deploy lands inside a subscribe round-trip: + /// prunes _subscribedTopics of the topics its new table no + /// longer carries, but an in-flight SUBSCRIBE records its own topic after that prune ran. + /// /// /// The connection's lifetime token; cancelled by its dispose. /// A task that completes when the re-subscribe has been answered. @@ -421,24 +430,21 @@ public sealed class MqttSubscriptionManager var filters = BuildFilters(live, _defaultQos); if (filters.Count == 0) { - // Silence here is how the wildcard defect hid: no exception, no log, no SUBSCRIBE. If any - // topic was live, resolving zero filters for it is an anomaly and must be loud. + // Silence here is how the wildcard defect hid: no exception, no log, no SUBSCRIBE. Live + // topics resolving to zero filters re-establishes nothing, which is the total-failure case + // by a different route, so it gets the total-failure answer — see the remarks. if (subscribed.Count > 0) { - _logger?.LogWarning( - "MQTT driver '{DriverId}': reconnect resolved NO filters for {TopicCount} live topic(s) " - + "({Topics}); those subscriptions are not being re-established.", - _driverId, - subscribed.Count, - string.Join(", ", subscribed)); - } - else - { - _logger?.LogDebug( - "MQTT driver '{DriverId}': reconnect had no live subscriptions to re-establish.", - _driverId); + throw new InvalidOperationException( + $"MQTT driver '{_driverId}': reconnect resolved NO filters for {subscribed.Count} live " + + $"topic(s) ({string.Join(", ", subscribed)}); tearing the session down rather than " + + "serving a connection that receives nothing."); } + _logger?.LogDebug( + "MQTT driver '{DriverId}': reconnect had no live subscriptions to re-establish.", + _driverId); + return; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs index 354caf4c..743c70bf 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs @@ -583,6 +583,51 @@ public sealed class MqttSubscriptionManagerTests mgr.Values.Read(OvenTempPath).StatusCode.ShouldBe(Good); } + /// + /// Driver.Mqtt-007. Live subscribed topics that resolve to ZERO filters re-establish + /// nothing, which is the same outcome as a total SUBACK failure and must get the same answer: + /// throw, so tears the fresh session down and retries rather than + /// serving a connected-but-deaf driver. A warning was not enough — the whole point of the type's + /// split contract is that "no subscription was re-established" is fatal to the session. + /// + /// + /// The state is reached the only way production can reach it: + /// prunes _subscribedTopics of the topics its new table no longer carries, but a SUBSCRIBE + /// already in flight records its topic after that prune has run — so a redeploy landing + /// inside the subscribe round-trip leaves a live topic no authored tag resolves to. + /// + [Fact] + public async Task OnReconnectedAsync_LiveTopicResolvingNoFilter_Throws_RatherThanServingADeafSession() + { + var transport = new FakeTransport(); + var mgr = NewManager(transport); + mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + + transport.DuringSubscribe = () => mgr.Register([]); // the redeploy lands mid-round-trip + await mgr.SubscribeAsync([OvenTempPath], TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + transport.DuringSubscribe = null; + + await Should.ThrowAsync( + () => mgr.OnReconnectedAsync(TestContext.Current.CancellationToken)); + } + + /// + /// The falsifying control: a reconnect with no live subscriptions at all re-establishes nothing + /// because there is nothing to establish, and must stay a no-op. Throwing here would tear down + /// the session of every driver whose tags have not been subscribed yet. + /// + [Fact] + public async Task OnReconnectedAsync_NoLiveSubscriptions_IsANoOp() + { + var transport = new FakeTransport(); + var mgr = NewManager(transport); + mgr.Register([Tag(OvenTempPath, $$"""{"topic":"{{OvenTopic}}","payloadFormat":"Scalar","dataType":"Float64"}""")]); + + await Should.NotThrowAsync(() => mgr.OnReconnectedAsync(TestContext.Current.CancellationToken)); + + transport.Calls.Count.ShouldBe(0); + } + // --------------------------------------------------------------------------------- // C1 — wildcard-authored tags on the paths that reconstruct state from a FILTER string. // Delivery matches an incoming topic (concrete); reconnect + degrade key off the subscribed @@ -898,10 +943,18 @@ public sealed class MqttSubscriptionManagerTests /// When set, the whole subscribe call throws instead of answering. public Exception? Throw { get; set; } + /// + /// Runs inside the subscribe round-trip, so a test can land a concurrent + /// in the window between its prune and this + /// call recording its topic as live. + /// + public Action? DuringSubscribe { get; set; } + public Task> SubscribeAsync( IReadOnlyList filters, CancellationToken cancellationToken) { Calls.Add(filters); + DuringSubscribe?.Invoke(); if (Throw is not null) throw Throw; IReadOnlyList outcomes = [.. filters.Select(f => new MqttSubscribeOutcome(f.Topic, !Reject(f.Topic), Reject(f.Topic) ? "NotAuthorized" : "GrantedQoS1"))]; From 3f85ba985ecdf4f59d519ea0377ffefecedfa0b7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 07:58:38 -0400 Subject: [PATCH 3/4] fix(mqtt): stop disposing the primary-host gate a live reconcile still holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driver.Mqtt-006 (Low, concurrency). DisposeAsync disposed _gate while StopAsync cancels the reconcile loop WITHOUT awaiting it, so a ReconcileAsync already in flight could be inside _gate.WaitAsync, or on its way to the finally-release, when the semaphore went away. The release then threw ObjectDisposedException naming SemaphoreSlim, logged as a reconcile failure — telling an operator nothing about what happened and hiding whatever the reconcile was really failing on. Left undisposed, matching MqttConnection's documented rule for its own lifecycle primitives and its stated rationale: a late callback racing teardown should be a benign no-op rather than an ObjectDisposedException naming the wrong type. Two lifecycle types in one module disagreeing about this is what gets copied into the third. Nothing is leaked — the gate holds no timer and no registration — and ClaimAsync still re-checks _disposed under it, so no claim can be created after dispose either way. Test is deterministic rather than timing-dependent: the gate is taken before the connect starts, so a MiniBroker in blackhole mode (TCP accepted, CONNECT never answered) accepting the socket proves the claim is inside the gate when dispose runs. RED: log.Captured contained "ObjectDisposedException". --- .../Sparkplug/SparkplugPrimaryHost.cs | 14 ++- .../SparkplugPrimaryHostTests.cs | 96 +++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugPrimaryHost.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugPrimaryHost.cs index 7911c872..02c1d696 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugPrimaryHost.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugPrimaryHost.cs @@ -73,7 +73,10 @@ public sealed class SparkplugPrimaryHost : IAsyncDisposable private readonly ILogger? _logger; private readonly TimeProvider _timeProvider; - /// Serializes claim/release against each other and against . + /// + /// Serializes claim/release against each other and against . Never + /// disposed — see for why. + /// private readonly SemaphoreSlim _gate = new(1, 1); private CancellationTokenSource? _loopCts; @@ -275,7 +278,14 @@ public sealed class SparkplugPrimaryHost : IAsyncDisposable ex, "MQTT driver '{DriverId}': Sparkplug primary-host stand-down failed during dispose.", _driverId); } - _gate.Dispose(); + // _gate is deliberately NOT disposed, matching MqttConnection's rule for its own lifecycle + // primitives. StopAsync cancels the reconcile loop without awaiting it, so a ReconcileAsync can + // still be inside the gate — or on its way to the finally-release — when this returns. Against a + // disposed semaphore that release throws an ObjectDisposedException naming SemaphoreSlim, which + // is logged as a reconcile failure and tells an operator nothing (and hides whatever the + // reconcile was really failing on). Left undisposed the late release is a harmless no-op: the + // gate holds no timer and no registration, and ClaimAsync re-checks _disposed under it, so no + // claim can be created after dispose either way. } private async Task ReconcileLoopAsync(CancellationToken ct) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugPrimaryHostTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugPrimaryHostTests.cs index 451a060c..0cffd5b5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugPrimaryHostTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugPrimaryHostTests.cs @@ -1,4 +1,5 @@ using System.Text; +using Microsoft.Extensions.Logging; using MQTTnet; using Shouldly; using Xunit; @@ -254,10 +255,105 @@ public sealed class SparkplugPrimaryHostTests await Should.NotThrowAsync(async () => await host.DisposeAsync()); } + /// + /// Driver.Mqtt-006. Dispose runs while a reconcile is in flight — the claim is inside the + /// gate, blocked on a broker that accepts TCP and never answers CONNECT — and must not leave that + /// reconcile releasing a disposed semaphore. The symptom is an ObjectDisposedException + /// naming SemaphoreSlim, logged as a reconcile failure: it tells an operator nothing about + /// what happened, and it hides whatever the reconcile was actually failing on. + /// + /// + /// Deterministic rather than timing-dependent: the gate is taken before the connect starts, so + /// the broker having accepted the socket proves the claim is inside it when dispose runs. + /// + [Fact] + public async Task Disposing_while_a_reconcile_is_in_flight_does_not_fault_on_a_disposed_gate() + { + using var broker = new MiniBroker { Blackhole = true }; + var log = new ExceptionCapturingLogger(); + var options = Options() with + { + Host = "127.0.0.1", + Port = broker.Port, + UseTls = false, + ProtocolVersion = MqttProtocolVersion.V311, + ConnectTimeoutSeconds = 2, + }; + var host = new SparkplugPrimaryHost(options, "drv-1", new FixedRole(shouldActAsPrimary: true), log); + + var starting = host.StartAsync(CancellationToken.None); + (await WaitUntilAsync(() => broker.AcceptedCount > 0, TimeSpan.FromSeconds(10))) + .ShouldBeTrue("the claim must be inside the gate before dispose runs"); + + await host.DisposeAsync(); + await starting; + + log.Captured.ShouldNotContain( + name => name == nameof(ObjectDisposedException), + "a teardown racing an in-flight reconcile must not surface a disposed-semaphore failure"); + } + + private static async Task WaitUntilAsync(Func condition, TimeSpan timeout) + { + var deadline = DateTime.UtcNow + timeout; + while (DateTime.UtcNow < deadline) + { + if (condition()) + { + return true; + } + + await Task.Delay(25); + } + + return condition(); + } + private sealed class FixedRole(bool shouldActAsPrimary) : IDriverPrimaryRoleSource { public bool ShouldActAsPrimary { get; private set; } = shouldActAsPrimary; public void PublishPrimaryRole(bool value) => ShouldActAsPrimary = value; } + + /// Records the type name of every exception logged, which is the observable here. + private sealed class ExceptionCapturingLogger : ILogger + { + private readonly List _captured = []; + + public IReadOnlyList Captured + { + get + { + lock (_captured) + { + return [.. _captured]; + } + } + } + + public IDisposable? BeginScope(TState state) + where TState : notnull + => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + if (exception is null) + { + return; + } + + lock (_captured) + { + _captured.Add(exception.GetType().Name); + } + } + } } From 2545bd7f7d776597539e5cd3390861133ff8490c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 07:59:34 -0400 Subject: [PATCH 4/4] docs(code-reviews): close Driver.Mqtt-006/-007 and Driver.Mqtt.Transport-002 Records the disposition and rationale for each: the primary-host gate is no longer disposed (matching MqttConnection's documented rule), a reconnect that resolves zero filters for live topics now throws like the zero-granted case it is indistinguishable from, and a password configured without a username is warned about rather than silently dropped. Both module headers drop to 0 open. --- .../Driver.Mqtt.Transport/findings.md | 17 ++++++++-- code-reviews/Driver.Mqtt/findings.md | 31 ++++++++++++++++--- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/code-reviews/Driver.Mqtt.Transport/findings.md b/code-reviews/Driver.Mqtt.Transport/findings.md index 8aa803d5..64c220a2 100644 --- a/code-reviews/Driver.Mqtt.Transport/findings.md +++ b/code-reviews/Driver.Mqtt.Transport/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-08-10 | | Commit reviewed | `b990adae` | | Status | Reviewed | -| Open findings | 1 | +| Open findings | 0 | ## Checklist coverage @@ -110,7 +110,7 @@ existing assertion depends on the branch being silent (713 tests green). | Severity | Low | | Category | Correctness & logic bugs | | Location | `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs:97-100` | -| Status | Open | +| Status | Resolved | **Description:** Credentials are attached only when `Username` is non-empty: `if (!string.IsNullOrEmpty(options.Username)) builder.WithCredentials(options.Username, options.Password ?? "")`. @@ -129,4 +129,15 @@ credential is visible; optionally send password-only credentials under `V500`, w permits it. Do **not** log the password itself — nothing on this path formats it today, and the options record's `PrintMembers` override exists to keep it that way. -**Resolution:** _(open)_ +**Resolution (2026-08-10):** Fixed `5d4dd17d` — the recommendation's first half only. A password +configured with a blank username now logs a warning saying the connection is being made +**anonymously** and naming the two ways to make that intentional; the credential is still not sent, +and the secret is never formatted into the message. The optional half (a password-only CONNECT +under `V500`) was deliberately **not** taken: 3.1.1 forbids it, so it cannot be the uniform +behaviour of a factory that serves both versions, and adding it under `V500` alone would change the +wire behaviour of deployments that connect happily today — the same "the explicit config stays +authoritative, say so rather than second-guess it" line Driver.Mqtt.Transport-001 settled on. +TDD: the warning assertion failed RED (`log.CountContaining("password") should be 1 but was 0`) +before the change. Paired with a falsifying control — a config carrying **neither** credential must +stay silent, or the fix would warn on every ordinary anonymous deployment and the first test would +still pass. Suite green (734). diff --git a/code-reviews/Driver.Mqtt/findings.md b/code-reviews/Driver.Mqtt/findings.md index c5dfd3de..e213e728 100644 --- a/code-reviews/Driver.Mqtt/findings.md +++ b/code-reviews/Driver.Mqtt/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-08-10 | | Commit reviewed | `b990adae` | | Status | Reviewed | -| Open findings | 2 | +| Open findings | 0 | ## Checklist coverage @@ -214,7 +214,7 @@ with a comment naming the extension method that needs it. Build verified. | Severity | Low | | Category | Concurrency & thread safety | | Location | `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugPrimaryHost.cs:278` | -| Status | Open | +| Status | Resolved | **Description:** `DisposeAsync` calls `_gate.Dispose()`. `StopAsync` cancels the reconcile loop but does not await it, so a `ReconcileAsync` already in flight can be sitting in @@ -236,7 +236,16 @@ stated rationale — the gate holds no timer and no surviving registration), or the reconcile loop before dispose returns. The former is the smaller change and the one already argued for in this module. -**Resolution:** _(open)_ +**Resolution (2026-08-10):** Fixed `3f85ba98` — the first option. `DisposeAsync` no longer disposes +`_gate`, and both the field and the dispose site now carry the rule and the reason, so the module +states **one** answer for its lifecycle primitives rather than two. Nothing is leaked (the gate +holds no timer and no registration) and `ClaimAsync` still re-checks `_disposed` under it, so no +claim can be created after dispose either way. TDD: a test that disposes while a reconcile is in +flight failed RED with `log.Captured should not contain an element satisfying (name == +"ObjectDisposedException") but does`. It is deterministic rather than timing-dependent — the gate is +taken *before* the connect starts, so a `MiniBroker` in blackhole mode (TCP accepted, CONNECT never +answered) having accepted the socket proves the claim is inside the gate when dispose runs. Suite +green (734). ### Driver.Mqtt-007 @@ -245,7 +254,7 @@ argued for in this module. | Severity | Low | | Category | Error handling & resilience | | Location | `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs:406-424` (`OnReconnectedAsync`) | -| Status | Open | +| Status | Resolved | **Description:** When the reconnect path finds live subscribed topics but resolves **zero** filters for them, it logs a warning and returns normally. The type's own contract, stated two paragraphs @@ -263,7 +272,19 @@ which is an argument for leaving the code alone, not for leaving the contract am *this* deaf-reconnect shape is exempt from the type's own rule. Do not leave the two answers side by side unexplained. -**Resolution:** _(open)_ +**Resolution (2026-08-10):** Fixed `2298485b` — it throws, and the remarks now state the single rule. +Exempting it was considered and rejected: the reason the partial-rejection case is exempt is that +tearing the session down would punish the tags still flowing, and here there are none — every live +topic is in the set that resolved to nothing — so the teardown costs nothing and the retry is not +futile, since it runs against whatever `Register` has published by then. The `subscribed.Count == 0` +branch stays a debug no-op: a reconnect with nothing subscribed yet re-establishes nothing because +there is nothing to establish. TDD: `Should.ThrowAsync` failed RED +(`should throw System.InvalidOperationException but did not`). The test reaches the state the only +way production can — `Register` prunes `_subscribedTopics` of the topics its new table dropped, but a +SUBSCRIBE already in flight records its own topic *after* that prune, so a deploy landing inside the +round-trip leaves a live topic no authored tag resolves to — driven through a `DuringSubscribe` hook +on the existing fake transport. Paired with the control that a reconnect with no live subscriptions +stays a no-op, which a blanket `filters.Count == 0` throw would have broken. Suite green (734). ### Driver.Mqtt-008