From 2d7dffcc420128c385e4c7a473b5b5bc13bea22f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 08:02:15 -0400 Subject: [PATCH] fix(mtconnect): volatile _options publication, IConvertible write guard, idempotent client dispose Driver.MTConnect-005/-006/-007, each TDD'd red-first. - -005: _options was the one field at the StartCoreAsync commit point published by a plain write and read lock-free from ReadAsync/WriteAsync/DiscoverAsync/HostName/health. Now Volatile.Write/Volatile.Read like _session/_binding/_announcedInstanceId, pinned by a source scan (MTConnectOptionsPublicationTests) that fails on any unfenced touch. - -006: TryFormatValue guarded null and Array, but Convert.ToString renders ANY non-IConvertible value as its type name under a Good status. Guard is now the property the remark relies on; the accepted set (string/bool/numeric/DateTime) is unchanged. - -007: MTConnectAgentClient.Dispose was a check-then-set on a non-volatile bool, so two racing disposals both passed and the loser cancelled an already-disposed CTS. Now an Interlocked.Exchange claim over an int, with the request seams reading it volatile. --- .../MTConnectAgentClient.cs | 28 +++-- .../MTConnectDriver.cs | 64 +++++++--- .../CannedAgentClient.cs | 8 ++ .../MTConnectAgentClientWriteTests.cs | 51 ++++++++ .../MTConnectOptionsPublicationTests.cs | 109 ++++++++++++++++++ .../MTConnectWriteTests.cs | 71 +++++++++--- 6 files changed, 290 insertions(+), 41 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectOptionsPublicationTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs index f40bf3e1..9bd36b8c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs @@ -82,7 +82,16 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable private readonly string _sampleQuerySuffix; private readonly TimeSpan _requestTimeout; private readonly TimeSpan _streamIdleTimeout; - private bool _disposed; + + /// + /// 1 once has claimed the teardown. An rather + /// than a bool so the claim is an : + /// a check-then-set let two concurrent disposals both pass, and the loser then cancelled a + /// the winner had already disposed + /// (Driver.MTConnect-007). Read with Volatile.Read at the request seams for the same + /// reason. + /// + private int _disposed; /// Creates a client for the Agent described by . /// The driver options carrying the Agent URI, device scope, and per-call deadline. @@ -157,7 +166,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable /// public async Task ProbeAsync(CancellationToken ct) { - ObjectDisposedException.ThrowIf(_disposed, this); + ObjectDisposedException.ThrowIf(Volatile.Read(ref _disposed) != 0, this); // ResponseContentRead (the default) buffers the whole body *under* this awaited, token-bound // call, so the parse below reads from memory — no network read can outlive the deadline. @@ -171,7 +180,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable /// public async Task CurrentAsync(CancellationToken ct) { - ObjectDisposedException.ThrowIf(_disposed, this); + ObjectDisposedException.ThrowIf(Volatile.Read(ref _disposed) != 0, this); using var response = await SendAsync(_currentUri, ct).ConfigureAwait(false); @@ -185,7 +194,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable long from, [EnumeratorCancellation] CancellationToken ct) { - ObjectDisposedException.ThrowIf(_disposed, this); + ObjectDisposedException.ThrowIf(Volatile.Read(ref _disposed) != 0, this); var uri = BuildSampleUri(from); @@ -257,7 +266,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable public async Task WriteObservationAsync( string deviceName, string dataItemId, string value, CancellationToken cancellationToken) { - ObjectDisposedException.ThrowIf(_disposed, this); + ObjectDisposedException.ThrowIf(Volatile.Read(ref _disposed) != 0, this); ArgumentException.ThrowIfNullOrWhiteSpace(deviceName); ArgumentException.ThrowIfNullOrWhiteSpace(dataItemId); ArgumentNullException.ThrowIfNull(value); @@ -371,15 +380,18 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable } /// + /// + /// Idempotent, including under a race: the teardown is claimed with a single atomic exchange, + /// so a second concurrent caller returns rather than cancelling an already-disposed + /// (Driver.MTConnect-007). + /// public void Dispose() { - if (_disposed) + if (Interlocked.Exchange(ref _disposed, 1) != 0) { return; } - _disposed = true; - // Cancel BEFORE disposing the clients so an in-flight read observes cancellation rather // than a disposed handler. _disposeCts.Cancel(); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 7734a187..58a946c6 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -292,6 +292,21 @@ public sealed class MTConnectDriver /// private long _announcedInstanceId; + /// + /// The options document currently in force. Installed at the StartCoreAsync commit + /// point alongside / / + /// , and read lock-free from ReadAsync, + /// WriteAsync, DiscoverAsync, HostName and the health paths. + /// + /// Published with Volatile.Write and read with Volatile.Read, exactly + /// like its siblings, so a reader that + /// takes this field BEFORE it takes — which both ReadAsync + /// and WriteAsync do — has the ordering that stops it judging a batch against the + /// previous document's Write.Enabled / RequestTimeoutMs paired with the new + /// tag table (Driver.MTConnect-005). Pinned by + /// MTConnectOptionsPublicationTests, which fails on any unfenced touch. + /// + /// private MTConnectDriverOptions _options; /// @@ -418,7 +433,7 @@ public sealed class MTConnectDriver // Sets _options AND the tag tables derived from it. No /probe model is available here (the // constructor dials nothing), so a raw tag whose blob declares no type falls back to String // until InitializeAsync rebuilds the binding against the Agent's own declaration. - _options = options; + Volatile.Write(ref _options, options); _binding = BuildBinding(options, probeModel: null); // Never null, so every consumer (Task 10's ReadAsync included) can ask it for a snapshot @@ -506,7 +521,7 @@ public sealed class MTConnectDriver internal Task? ProbeLoopTask => Volatile.Read(ref _probeTask); /// The options currently in force — the constructor's, or the last document applied. - internal MTConnectDriverOptions EffectiveOptions => _options; + internal MTConnectDriverOptions EffectiveOptions => Volatile.Read(ref _options); /// /// @@ -581,7 +596,7 @@ public sealed class MTConnectDriver await StopSampleStreamAsync(cancellationToken).ConfigureAwait(false); await StopProbeLoopAsync(cancellationToken).ConfigureAwait(false); - if (existing is null || RequiresClientRebuild(_options, incoming)) + if (existing is null || RequiresClientRebuild(Volatile.Read(ref _options), incoming)) { await TeardownCoreAsync().ConfigureAwait(false); await StartCoreAsync(incoming, existingClient: null, cancellationToken).ConfigureAwait(false); @@ -686,7 +701,7 @@ public sealed class MTConnectDriver // One read of the session reference: the client used below is the one that was live at this // instant, never a half-swapped pairing (see the _session remarks). var client = Volatile.Read(ref _session)?.Client; - var options = _options; + var options = Volatile.Read(ref _options); // Read once, for the same reason the session is: every reference in this batch must be // resolved against ONE tag table, not half against the table a concurrent re-initialize @@ -794,7 +809,7 @@ public sealed class MTConnectDriver // Read once each, for the same reason ReadAsync does: every slot in this batch is judged // against ONE options document and ONE tag table, never half against a table a concurrent // re-initialize just replaced. - var options = _options; + var options = Volatile.Read(ref _options); var client = Volatile.Read(ref _session)?.Client; var binding = Volatile.Read(ref _binding); @@ -895,12 +910,18 @@ public sealed class MTConnectDriver /// it is what an MTConnect consumer will see verbatim. /// /// - /// Arrays are refused rather than rendered. + /// Anything that is not is refused rather than rendered. /// falls back to /// for a non- value, so an array /// would be written as the literal text System.Double[] — which the Agent stores /// happily, making the corruption invisible until someone reads the observation back. - /// TIME_SERIES / DATA_SET writes are out of scope for v1. + /// The guard is therefore written against that property itself rather than against + /// , the one type it was first noticed on: an + /// Opc.Ua.ExtensionObject, an XmlElement or a List<T> from a + /// non-SDK caller corrupts an observation in exactly the same way (Driver.MTConnect-006). + /// string, bool, and every numeric type ARE + /// , so the accepted set is unchanged. TIME_SERIES / DATA_SET + /// writes are out of scope for v1. /// /// /// Every formatting throw is caught, not just the documented conversion ones. The @@ -916,8 +937,10 @@ public sealed class MTConnectDriver text = string.Empty; // A PUT query has no null to send, and the empty string is a real observation rather than an - // absence — so "no value" is refused instead of being silently turned into one. - if (value is null or Array) + // absence — so "no value" is refused instead of being silently turned into one. Everything + // that is not IConvertible is refused with it: Convert.ToString renders such a value as its + // TYPE NAME, which the Agent stores under a Good status (see the remarks). + if (value is null or Array or not IConvertible) { return false; } @@ -1202,7 +1225,7 @@ public sealed class MTConnectDriver // to another. // Both read once, together: a concurrent re-initialize must not scope half this walk to one // device and half to another, nor stamp half its leaves writable and half not. - var options = _options; + var options = Volatile.Read(ref _options); var deviceScope = options.DeviceName?.Trim(); var writeEnabled = options.Write.Enabled; @@ -1422,7 +1445,7 @@ public sealed class MTConnectDriver /// host:port convention: several MTConnect drivers in one server disambiguate on the /// Admin dashboard by endpoint rather than by driver-instance id. /// - public string HostName => _options.AgentUri; + public string HostName => Volatile.Read(ref _options).AgentUri; /// /// @@ -1796,12 +1819,14 @@ public sealed class MTConnectDriver // that is not running; without this, the resubscribe returns a handle, the caller is // told SubscriptionEstablished, and the next successful read reports a perfectly green // driver over a subscription that can never deliver a value (#485). + var agentUri = Volatile.Read(ref _options).AgentUri; + _streamDegraded = true; - Degrade($"The MTConnect Agent at '{_options.AgentUri}' does not serve a framed /sample stream; subscriptions cannot deliver until the driver is re-initialized."); + Degrade($"The MTConnect Agent at '{agentUri}' does not serve a framed /sample stream; subscriptions cannot deliver until the driver is re-initialized."); _logger.LogWarning( "MTConnect driver {DriverInstanceId} refused to start a /sample stream for {AgentUri}: the endpoint was already found not to stream. The subscription is registered but will receive NO updates until the driver is re-initialized.", - _driverInstanceId, _options.AgentUri); + _driverInstanceId, agentUri); return; } @@ -1828,7 +1853,7 @@ public sealed class MTConnectDriver // _subscriberFaultLogged). Interlocked.Exchange(ref _subscriberFaultLogged, 0); - var options = _options; + var options = Volatile.Read(ref _options); var cts = new CancellationTokenSource(); _pumpCts = cts; _pumpTask = Task.Run( @@ -2497,7 +2522,8 @@ public sealed class MTConnectDriver MTConnectProbeModel model; try { - model = await BoundedAsync(session.Client.ProbeAsync, "/probe", _options.RequestTimeoutMs, cancellationToken) + model = await BoundedAsync( + session.Client.ProbeAsync, "/probe", Volatile.Read(ref _options).RequestTimeoutMs, cancellationToken) .ConfigureAwait(false); } catch (ObjectDisposedException ex) @@ -2652,7 +2678,7 @@ public sealed class MTConnectDriver .ConfigureAwait(false); // ---- commit point: everything below is non-throwing bookkeeping ---- - _options = options; + Volatile.Write(ref _options, options); // Derived from the options AND the /probe model just fetched: a raw tag whose TagConfig // blob declares no type takes the type the Agent itself declares for that DataItem, so a @@ -2904,7 +2930,7 @@ public sealed class MTConnectDriver { return HasConfigBody(driverConfigJson) ? ParseOptions(_driverInstanceId, driverConfigJson) - : _options; + : Volatile.Read(ref _options); } catch (Exception ex) { @@ -3013,7 +3039,7 @@ public sealed class MTConnectDriver { // A bare "A task was canceled" names neither the Agent nor the request that stalled. throw new TimeoutException( - $"MTConnect {request} request for driver '{_driverInstanceId}' against '{_options.AgentUri}' did not complete within {timeoutMs} ms."); + $"MTConnect {request} request for driver '{_driverInstanceId}' against '{Volatile.Read(ref _options).AgentUri}' did not complete within {timeoutMs} ms."); } } @@ -3050,7 +3076,7 @@ public sealed class MTConnectDriver _logger.LogWarning( ex, "MTConnect driver {DriverInstanceId} could not read /current from {AgentUri}; the batch is reported Bad.", - _driverInstanceId, _options.AgentUri); + _driverInstanceId, Volatile.Read(ref _options).AgentUri); } /// Publishes without downgrading a Faulted driver. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs index d6627661..3e752354 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs @@ -370,6 +370,13 @@ internal sealed class CannedAgentClient : IMTConnectAgentClient, IDisposable /// When set, a write throws this instead of answering — the transport-failure leg. public Exception? WriteFailure { get; set; } + /// + /// Ran after a write has been recorded and before its outcome is returned. The deterministic + /// injection point for something that must happen BETWEEN two slots of one batch — cancelling + /// the batch's token, for instance — with no timing hope involved. + /// + public Action? AfterWrite { get; set; } + /// public Task WriteObservationAsync( string deviceName, string dataItemId, string value, CancellationToken cancellationToken) @@ -377,6 +384,7 @@ internal sealed class CannedAgentClient : IMTConnectAgentClient, IDisposable ObjectDisposedException.ThrowIf(IsDisposed, this); cancellationToken.ThrowIfCancellationRequested(); Writes.Enqueue((deviceName, dataItemId, value)); + AfterWrite?.Invoke(); return WriteFailure is null ? Task.FromResult(WriteOutcome) : throw WriteFailure; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs index ea7d20ec..b431eccb 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Net; using System.Text; using Shouldly; @@ -353,4 +354,54 @@ public sealed class MTConnectAgentClientWriteTests await Should.ThrowAsync( () => client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None)); } + + /// + /// Driver.MTConnect-007 — concurrent disposals must not throw out of Dispose. + /// + /// + /// + /// A plain check-then-set on a non-volatile bool lets two racing disposals both pass + /// the guard; the loser then calls Cancel() on a + /// the winner has already disposed, which throws out of + /// Dispose. The driver disposes this client under its lifecycle lock and through + /// SafeDispose, so the throw is caught and traced today — but "the caller happens to + /// catch it" is not idempotence, and nothing on this type says disposal is single-threaded. + /// + /// + /// The window is a handful of instructions, so one pass proves nothing — the racers + /// are released together on a barrier and the whole thing is repeated. Against the + /// unguarded field this fails within a few hundred rounds (observed: round 690); once the + /// guard is an no schedule can fail it, so + /// it cannot flake either way. + /// + /// + [Fact] + public async Task Concurrent_disposals_are_idempotent() + { + const int racers = 8; + + for (var round = 0; round < 3000; round++) + { + var client = new MTConnectAgentClient(Options(), new StubHandler((_, _) => Task.FromResult(Ok()))); + using var start = new Barrier(racers); + var failures = new ConcurrentBag(); + + await Task.WhenAll(Enumerable.Range(0, racers).Select(_ => Task.Run(() => + { + start.SignalAndWait(); + try + { + client.Dispose(); + } + catch (Exception ex) + { + failures.Add(ex); + } + }))); + + failures.ShouldBeEmpty( + $"Dispose must be idempotent under a race (round {round}); " + + string.Join(" | ", failures.Select(e => e.GetType().Name))); + } + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectOptionsPublicationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectOptionsPublicationTests.cs new file mode 100644 index 00000000..2596f477 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectOptionsPublicationTests.cs @@ -0,0 +1,109 @@ +using System.Text.RegularExpressions; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests; + +/// +/// Driver.MTConnect-005 — MTConnectDriver._options must be published and read the way its +/// three siblings at the same commit point are (_session, _binding, +/// _announcedInstanceId): Volatile.Write once, Volatile.Read everywhere else. +/// +/// +/// +/// Why a source scan and not a behavioural test. The defect is a missing memory fence: +/// a plain field write publishes the new options document with no release, and a reader that +/// reads _options BEFORE it reads _binding — which both ReadAsync and +/// WriteAsync do — has no acquire ordering between the two, so it can in principle +/// judge a batch against the previous Write.Enabled / RequestTimeoutMs paired +/// with the new tag table. No test can force that interleaving on x86/arm64 in bounded time; +/// what IS checkable, and what actually regresses, is whether a new read site was written +/// with the fence. This is the same shape as +/// LibplctagGatewayConsumptionGuardTests: the defect recurs by being reintroduced at a +/// call site no per-site test covers. +/// +/// +/// The floor below exists so the scan cannot go blind. If the count drops, the field was +/// renamed or the sites were consolidated — re-derive the floor from the code; never lower it +/// to make this pass. +/// +/// +[Trait("Category", "Unit")] +public sealed class MTConnectOptionsPublicationTests +{ + /// Any mention of the field, so an unguarded read is caught as well as an unguarded write. + private static readonly Regex Mention = new(@"_options\b", RegexOptions.Compiled); + + /// The two guarded forms. Nothing else may touch the field. + private static readonly Regex Guarded = + new(@"Volatile\.(Read|Write)\(ref _options[,)]", RegexOptions.Compiled); + + /// The declaration itself, which is neither a read nor a write. + private static readonly Regex Declaration = + new(@"^\s*private\s+MTConnectDriverOptions\s+_options\s*;\s*$", RegexOptions.Compiled); + + [Fact] + public void Every_touch_of_the_shared_options_field_goes_through_Volatile() + { + var source = File.ReadAllLines(Path.Combine( + LocateRepoRoot(), "src", "Drivers", "ZB.MOM.WW.OtOpcUa.Driver.MTConnect", "MTConnectDriver.cs")); + + var offending = new List(); + var guarded = 0; + + for (var i = 0; i < source.Length; i++) + { + var line = source[i]; + var code = StripComment(line); + + if (!Mention.IsMatch(code) || Declaration.IsMatch(code)) + { + continue; + } + + var mentions = Mention.Matches(code).Count; + var fenced = Guarded.Matches(code).Count; + guarded += fenced; + + if (fenced < mentions) + { + offending.Add($"line {i + 1}: {line.Trim()}"); + } + } + + offending.ShouldBeEmpty( + "MTConnectDriver._options is shared state read lock-free from the read, write, discovery " + + "and health paths while a re-initialize replaces it. Publish it with Volatile.Write and " + + "read it with Volatile.Read, exactly as _session / _binding / _announcedInstanceId are " + + $"(Driver.MTConnect-005). Unfenced: {string.Join(" | ", offending)}"); + + guarded.ShouldBeGreaterThanOrEqualTo(12); + } + + /// Removes a trailing line comment and skips XML-doc lines, which mention the field by name. + private static string StripComment(string line) + { + var trimmed = line.TrimStart(); + if (trimmed.StartsWith("//", StringComparison.Ordinal) || trimmed.StartsWith("*", StringComparison.Ordinal)) + { + return string.Empty; + } + + var comment = line.IndexOf("//", StringComparison.Ordinal); + + return comment >= 0 ? line[..comment] : line; + } + + private static string LocateRepoRoot() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx"))) + { + dir = dir.Parent; + } + + dir.ShouldNotBeNull("Could not locate the repo root (ZB.MOM.WW.OtOpcUa.slnx) above the test assembly."); + + return dir.FullName; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs index 936f3b6b..146330e9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs @@ -191,6 +191,57 @@ public sealed class MTConnectWriteTests client.Writes.ShouldBeEmpty(); } + [Fact] + public async Task A_non_convertible_composite_value_is_refused_as_a_type_mismatch() + { + // Driver.MTConnect-006. The array guard's stated REASON is "Convert.ToString falls back to + // object.ToString for a non-IConvertible value", and an array is only one shape carrying that + // property. A List from a non-SDK caller (or an Opc.Ua.ExtensionObject, or an XmlElement) + // renders as its TYPE NAME, which the Agent stores under a Good status — the identical + // invisible corruption the array guard exists to prevent, refused nowhere. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync( + [new WriteRequest(PositionRawPath, new List { 1.0, 2.0 })], + Ct); + + results[0].StatusCode.ShouldBe(BadTypeMismatch); + client.Writes.ShouldBeEmpty(); + } + + [Theory] + [InlineData((byte)7, "7")] + [InlineData('x', "x")] + [InlineData("already text", "already text")] + public async Task An_IConvertible_scalar_still_writes(object value, string expected) + { + // Falsification control for the guard above: the accepted set must be UNCHANGED. Every scalar + // this driver can carry is IConvertible, so a refusal expressed as "not a primitive I + // recognise" — rather than as the IConvertible property the remark relies on — would silently + // start refusing real writes instead of composite ones. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, value)], Ct); + + results[0].StatusCode.ShouldBe(Good); + client.Writes.ShouldHaveSingleItem().Value.ShouldBe(expected); + } + + [Fact] + public async Task A_DateTime_still_writes_as_its_invariant_string() + { + // DateTime is IConvertible but is neither a primitive nor a string — the shape most likely to + // be swept up by a hand-rolled "is this a scalar?" guard. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync( + [new WriteRequest(PositionRawPath, new DateTime(2026, 8, 10, 13, 45, 0, DateTimeKind.Utc))], + Ct); + + results[0].StatusCode.ShouldBe(Good); + client.Writes.ShouldHaveSingleItem().Value.ShouldBe("08/10/2026 13:45:00"); + } + [Fact] public async Task A_value_whose_own_formatter_throws_costs_its_slot_and_nothing_more() { @@ -358,16 +409,19 @@ public sealed class MTConnectWriteTests // The fill must start at the cancelled slot, not overwrite the batch. Slot 0 completed against // the Agent before the token was cancelled, and reporting it BadCommunicationError would tell // an operator a write failed that actually landed — the one thing worse than reporting a - // failure. Cancellation is injected from inside slot 1's own formatting, so the ordering is a - // fact of the run rather than a timing hope. + // failure. Cancellation is injected from inside slot 0's own Agent call, so the ordering is a + // fact of the run rather than a timing hope. (It used to be injected from a value's ToString; + // such a value is not IConvertible and is now refused before it is ever rendered — see + // Driver.MTConnect-006 — so the injection point moved to the seam it is asserting around.) var (driver, client) = await WritableDriverAsync(); using var cts = new CancellationTokenSource(); + client.AfterWrite = () => cts.Cancel(); var results = await driver.WriteAsync( [ new WriteRequest(PositionRawPath, 42.5), - new WriteRequest(PositionRawPath, new CancellingFormatter(cts)), new WriteRequest(PositionRawPath, 7.5), + new WriteRequest(PositionRawPath, 9.5), ], cts.Token); @@ -380,17 +434,6 @@ public sealed class MTConnectWriteTests client.Writes.ShouldHaveSingleItem().Value.ShouldBe("42.5"); } - /// Cancels the batch from inside its own rendering, then renders normally. - private sealed class CancellingFormatter(CancellationTokenSource cts) - { - public override string ToString() - { - cts.Cancel(); - - return "99"; - } - } - [Fact] public async Task One_bad_request_does_not_fail_the_others_in_the_batch() {