fix(transport): detect a physically dropped serial/TCP connection so ConnectionStatus.Lost is reachable

[tylerkron]

Closes #382.

The problem, plainly

Pull the USB cable out of a DAQiFi box and Core carries on as if nothing happened. IsConnected stays true, no event fires, and the app keeps showing a live device that silently returns no data — indefinitely. That is the single most common way a DAQ connection ends, and it was the one Core could not see.

ConnectionStatus.Lost already existed and DaqifiDevice was already fully wired to raise it. Nothing could ever trigger it on serial, because three layers each swallowed the evidence:

1. SerialStreamTransport.IsConnected is SerialPort.IsOpen — the OS handle, which stays open after the device vanishes.
2. The reader loop caught a failed read, raised its own error event, slept 100 ms, and carried on. The transport was never told.
3. The writer loop logged a failed write as a warning and kept draining the queue. Also never told.

So OnStatusChanged(false, …) was only ever raised by DisconnectAsync — an intentional disconnect — and the Status = Lost branch was dead code for serial.

What a user experiences now

"I unplugged it." Within about three seconds the device reports ConnectionStatus.Lost and IsConnected goes false — measured at 1.6 s on the bench with a real cable pull. If data was flowing, it is reported in well under a second. The registry prunes it, so a later reconnect under the same key opens a real connection instead of handing back the dead handle.

"I clicked disconnect." Still Disconnected, never Lost. The two remain cleanly distinguishable — that is what makes Lost actionable for #379's auto-reconnect.

"I'm on Windows / Linux / macOS." Identical behavior. No WMI, no Win32_DeviceChangeEvent watcher, no OS-specific code path — which is the point, since the desktop app had to write exactly that outside Core.

How it works

Two independent detectors, sharing one internal TransportConnectionWatchdog so that "declare this connection lost, exactly once, whichever signal wins" lives in a single place.

Transport	Drop	Reported within
Serial	Cable unplugged / device re-enumerated	~3 s — port-presence poll, 1 s cadence, 2 consecutive misses
Serial	Reads or writes failing while traffic flows	< 1 s — 5 consecutive I/O failures, no successful transfer between them
TCP	Peer closed / connection reset	< 1 s — same I/O fault escalation
TCP	Link silently severed (power loss, WiFi drop)	~20 s — TCP keep-alive (10 s idle, 3 s probes, 3 retries)

Port presence is SerialPort.GetPortNames() containing the port, falling back to the device node still existing on Unix. Requiring both to say "absent" means a transient enumeration hiccup cannot disconnect anything. It is armed only if the probe can see the port at connect time, so a platform or port-name spelling the probe does not recognize disables the check instead of reporting a false drop on every poll — fault escalation still covers that port.

I/O fault escalation is the new public ITransportHealthSink. StreamMessageConsumer and MessageProducer take one optionally and report every read/write outcome; DaqifiDevice passes the transport when it implements it. A single failure escalates nothing — the threshold is a run of five with no successful transfer in between, and any successful read or write resets the run. Timeouts are not failures at all: an idle read timeout is what a quiet device looks like, and a write timeout means the device is not draining its buffer right now, not that it is gone.

Everything is additive. IStreamTransport is unchanged, so the five test doubles implementing it (and any consumer's) still compile; a consumer that passes no health sink gets exactly the old behavior.

The TCP transport (acceptance item)

Checked, and it had the same gap — worse in one way. TcpClient.Connected describes the last completed operation, and nothing raised StatusChanged(false) for an unexpected drop, so ConnectionStatus.Lost was unreachable over TCP too. Fixed alongside:

• The same fault escalation, including a zero-byte socket read — which on a NetworkStream unambiguously means the peer closed, and which the reader loop previously treated as "no data yet" and spun on forever. Only NetworkStream is escalated this way; other stream types legitimately return 0 for "nothing right now".
• TCP keep-alive for the case escalation cannot reach: a link severed with no FIN or RST, where reads would otherwise just keep timing out. The knobs are best-effort — a platform that rejects one leaves the connection working without it.

Hardware validation

Bench Nyquist 1, fw 3.7.2, macOS, /dev/cu.usbmodem1101, example CLI built against this branch.

Scenario	Result
70 s streaming, 100 Hz, channels 0+1	5555 samples, exit 0. No false Lost, no spurious StatusChanged — the only status event all session was Disconnected at teardown
90 s near-idle, 1 Hz, channel 0 (the case the liveness poll dominates)	71 samples, exit 0. No false Lost
Intentional disconnect	Status: Disconnected on every run, never Lost
Throughput overhead of the 1 Hz poll	None measurable — 79.4 % of requested rate in both runs, exactly the pre-existing fw 3.7.2 clock ratio (fw#716)
Reconnect after disconnect	Clean, immediately, every run

✅ Physical unplug — validated, cable actually pulled

Run with the committed SerialUnplugValidationTests harness on the bench Nyquist 1 (fw 3.7.2, macOS, /dev/cu.usbmodem1101, net9.0). The harness watches the device node itself, so the latency below is measured from the moment the port actually vanished — not from the operator prompt.

[16:07:50.572Z] StatusChanged: Connecting
[16:07:50.579Z] StatusChanged: Connected
[16:08:10.585Z] No false positives during the healthy window.   (20 s warm-up, zero status changes)
[16:08:28.580Z] StatusChanged: Lost
[16:08:28.581Z] Port vanished from the system at 16:08:26.970Z
[16:08:28.581Z] ConnectionStatus.Lost observed at 16:08:28.580Z
[16:08:28.581Z] DETECTION LATENCY: 1610 ms
[16:08:28.584Z] PASS — unplug reported as Lost, not Disconnected.

1610 ms from the port disappearing to ConnectionStatus.Lost — comfortably inside the documented ~3 s bound, and consistent with the mechanism (a 1 s poll cadence needing 2 consecutive misses, with the unplug landing part-way through an interval). It reported Lost, never Disconnected; the trailing Disconnected is test-teardown disposal, after the assertion. The 20 s warm-up immediately beforehand produced zero status changes, so the same run demonstrates both halves: it fires when it should and stays silent when it shouldn't.

Second run, cable replugged and pulled again, re-verifying PR #381's stale-registration pruning against a real unplug:

[16:10:34.148Z] Device stopped reporting IsConnected after 16.04s.
[16:10:34.214Z] PASS — the unplugged device was pruned from the registry.

PruneDisconnected() returned 1 and the registry emptied, with DeviceRemovalReason.Disconnected — the pruning that #381 documented as unreachable for an unplugged USB device now works. The 16.04 s in that line is not a detection latency: unlike run 1, that timer starts at the operator prompt, so it is dominated by human reaction time before the cable ever moved. Run 1's 1610 ms is the real measurement.

To re-run either (each needs its own unplug — replug in between):

DAQIFI_UNPLUG_PORT=/dev/cu.usbmodem1101 \
  dotnet test src/Daqifi.Core.Tests/Daqifi.Core.Tests.csproj -f net9.0 \
  --filter "FullyQualifiedName~UnpluggedSerialDevice_ReportsConnectionLost" \
  --logger "console;verbosity=detailed"

DAQIFI_UNPLUG_PORT=/dev/cu.usbmodem1101 \
  dotnet test src/Daqifi.Core.Tests/Daqifi.Core.Tests.csproj -f net9.0 \
  --filter "FullyQualifiedName~UnpluggedSerialDevice_IsPrunedFromTheRegistry" \
  --logger "console;verbosity=detailed"

Without DAQIFI_UNPLUG_PORT both log why they did nothing and return, so CI is unaffected.

Testing

42 new unit tests plus the 2 hardware-gated ones. Full suite green on net9.0 and net10.0 (1976 Core + 23 MCP), run three times to check the new background timer introduced no flakiness in the timing-sensitive tests. 0 warnings.

The escalation and the "recoverable blip" are covered at three levels: the decision logic directly (TransportConnectionWatchdogTests), the reader/writer loops against a stream that starts failing and then recovers, and end to end over a real loopback socket where killing the server produces ConnectionStatus.Lost on a real DaqifiDevice — and an intentional Disconnect() on that same setup produces Disconnected with no Lost.

Acceptance criteria

• Unplugged serial device transitions to Lost within a bounded, documented time, raising StatusChanged — measured at 1610 ms on real hardware against the documented ~3 s bound (docs/DEVICE_INTERFACES.md and XML docs)
• Works on macOS, Windows, Linux (no WMI) — GetPortNames() + Unix device-node fallback, no OS-specific branch
• An intentional Disconnect() still reports Disconnected, not Lost — unit tests + every bench run; the unplug run reported Lost and never Disconnected
• Unit tests with a stream that starts failing, covering both the escalation and the recoverable blip
• Verified on real hardware with an actual unplug — cable physically pulled, log above
• TCP transport checked for the same gap — it had it, fixed alongside (see above)
• feat(device): connected-device registry with duplicate-across-transport detection #381's registry pruning re-verified against a real unplug — second cable pull, device pruned with DeviceRemovalReason.Disconnected. The "know this limit" note is removed from docs/DEVICE_INTERFACES.md and DaqifiDeviceRegistry's XML docs, replaced by the detection-bound table — and that replacement text is now backed by the measurement rather than by intent

Scope

Detection only. Automatic reconnect after Lost stays with #379, which this unblocks. On a detected drop the transport closes its handle but the device's consumer/producer are left running — tearing them down from inside the event would re-enter the very reader thread that raised it; the docs say to call Disconnect() off the callback instead.

Sibling PR note: #399 is also editing SerialStreamTransport, bounding WriteTimeout and adding an SD-download deadline. No semantic overlap — this PR owns drop detection and adds no SD-path timeouts — but expect a textual conflict in the constructor and field block.

Not merging — opened for review.

🤖 Generated with Claude Code

[tylerkron]

/agentic_review

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Misses survive probe exception ✓ Resolved 🐞 Bug ≡ Correctness

Description

TransportConnectionWatchdog.PollPresence() keeps the current presence-miss run when the probe
throws, so a miss → exception → miss sequence can incorrectly satisfy the “consecutive misses”
threshold and signal a lost connection. This violates the intended semantics that probe exceptions
are “no observation” and should not contribute to (or preserve) an absence run.

Code

src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[R255-261]

+        catch
+        {
+            // A probe that throws is a failure to observe, not evidence of a drop: never let it
+            // disconnect a healthy transport, and never let it kill the timer thread. Probes are
+            // required to surface "could not answer" this way rather than returning false, which
+            // would be indistinguishable from the device having actually gone.
+        }

Relevance

●●● Strong

Repo strongly favors “probe exceptions aren’t evidence” semantics and robustness (PR #384
timeout/idle treated benignly); likely reset miss run.

PR-#384

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PollPresence() increments _presenceMisses on probe() == false and only resets it on `probe()
== true; when probe()` throws, the catch block is empty, so the prior miss count persists across
the exception and can incorrectly reach PresenceMissThreshold on the next observed miss.

src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[221-266]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`TransportConnectionWatchdog.PollPresence()` treats a probe exception as “failure to observe” (by swallowing it), but it does not clear `_presenceMisses`. This allows `_presenceMisses` to carry across an exception, so two `false` results separated by an exception can still be counted as “consecutive misses” and trigger a false loss.

### Issue Context
Presence polling is meant to require *consecutive observed absences*. An exception is explicitly not an observed absence, so it should break the miss run.

### Fix Focus Areas
- src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[221-266]

### Suggested fix
- In the `catch` of `PollPresence()`, clear the miss run (e.g., `Volatile.Write(ref _presenceMisses, 0);`).
- Add/extend a regression test to cover the specific sequence: `probe returns false` → `probe throws` → `probe returns false`, and assert **no** loss is signaled (i.e., the exception breaks the consecutive-miss run).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Probe errors trigger false loss ✓ Resolved 🐞 Bug ≡ Correctness

Description

SerialStreamTransport.IsPortEnumerated() swallows SerialPort.GetPortNames()/File.Exists() exceptions
and returns false, so two transient probe failures are treated as two consecutive “misses” and can
incorrectly escalate to a lost connection.
This can close a healthy serial connection and raise StatusChanged(false, ...) even though the probe
never actually observed the port disappearing.

Code

src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[R398-427]

+        try
+        {
+            foreach (var name in SerialPort.GetPortNames())
+            {
+                if (string.Equals(name, portName, StringComparison.OrdinalIgnoreCase))
+                {
+                    return true;
+                }
+            }
+        }
+        catch (Exception)
+        {
+            // Enumeration can fail transiently (a /dev scan racing a device change, a registry
+            // read on Windows). Fall through to the filesystem check rather than concluding the
+            // port has gone away.
+        }
+
+        try
+        {
+            if (portName.StartsWith('/'))
+            {
+                return File.Exists(portName);
+            }
+        }
+        catch (Exception)
+        {
+            // Same reasoning: an unreadable path is not evidence the device was unplugged.
+        }
+
+        return false;

Relevance

●●● Strong

Team previously accepted transport-hardening to avoid misleading transient failures and preserve
exceptions (PR #240, #237).

PR-#240
PR-#237

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

StartDropDetection arms presence polling using IsPortPresent(), which calls IsPortEnumerated().
IsPortEnumerated() catches enumeration/filesystem exceptions and then returns false, and
PollPresence() treats a false probe result as a miss and signals loss after two misses
(PresenceMissThreshold=2). Because the exceptions are swallowed, the watchdog’s “probe throws => no
evidence” safeguard never applies.

src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[353-384]
src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[386-428]
src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[44-55]
src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[221-254]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SerialStreamTransport.IsPortEnumerated()` currently catches exceptions from `SerialPort.GetPortNames()` and from the Unix filesystem fallback, but then falls through to `return false;`. This converts “probe failed” into “port absent”, which allows `TransportConnectionWatchdog` presence polling to count those as real misses and declare the connection lost.

### Issue Context
- Presence polling declares loss after `PresenceMissThreshold` consecutive misses.
- `TransportConnectionWatchdog.PollPresence()` intentionally treats a *throwing* probe as “no evidence” (it catches and does nothing), but `IsPortEnumerated()` prevents that by swallowing exceptions and returning `false`.

### Fix Focus Areas
- src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[386-428]
- src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[353-375]
- src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[221-254]

### Suggested fix
Implement “inconclusive” semantics for probe failures so they do not count as absence:
- Option A (minimal): in `IsPortEnumerated`, if `SerialPort.GetPortNames()` throws, return `true` (treat as present/unknown) so polling does not increment misses. Similarly, if the Unix `File.Exists` check throws, return `true`.
- Option B (cleaner): allow exceptions to propagate out of `IsPortEnumerated` (remove the catches) and wrap the *connect-time* `IsPortPresent()` call inside `StartDropDetection()` with a try/catch that simply disables polling if the probe can’t run. This lets `TransportConnectionWatchdog.PollPresence()`’s existing `catch` correctly treat probe failures as “no evidence” (no miss increment).

Add/adjust a unit test around the presence probe path to ensure repeated probe failures do not cause a loss (e.g., by exercising `PortPresenceProbe` throwing, or by refactoring `IsPortEnumerated` to make the exception path injectable/testable).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Unsynchronized timestamp publication ✓ Resolved 🐞 Bug ☼ Reliability

Description

PortDisappearanceWatcher writes _portGoneAtUtc from a background thread and exposes it without
synchronization, so the main test can observe a stale/null value and log an incorrect disappearance
time. This can mislead the printed latency measurement (and the branch that chooses between “port
disappeared” vs “never disappeared”).

Code

src/Daqifi.Core.Tests/Communication/Transport/SerialUnplugValidationTests.cs[R263-274]

+                    try
+                    {
+                        if (!SerialStreamTransport.IsPortEnumerated(portName))
+                        {
+                            _portGoneAtUtc = DateTime.UtcNow;
+                            return;
+                        }
+                    }
+                    catch (Exception)
+                    {
+                        // The probe could not answer this time; that is not the port going away.
+                        // Keep watching rather than recording a bogus disappearance time.

Relevance

●● Moderate

Team accepts race/test-hardening changes (PR #237 deterministic tests; PR #240 concurrent disconnect
hardening), but no exact pattern found.

PR-#237
PR-#240

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The watcher thread sets _portGoneAtUtc when the port is first observed absent, while
PortGoneAtUtc returns the field directly with no synchronization, creating a cross-thread
publication race.

src/Daqifi.Core.Tests/Communication/Transport/SerialUnplugValidationTests.cs[251-295]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`PortDisappearanceWatcher` updates `_portGoneAtUtc` on a dedicated thread, but the value is read from the main test thread without any lock/Volatile/Interlocked publication. This is a data race and can produce stale/null reads in the diagnostic output.

### Issue Context
This is test-only code, but it is intended to produce trustworthy latency measurements and messaging.

### Fix Focus Areas
- src/Daqifi.Core.Tests/Communication/Transport/SerialUnplugValidationTests.cs[251-295]

### Suggested fix
Use one of:
- A private lock guarding both write and read of `_portGoneAtUtc`.
- Store ticks in a `long` field (e.g., `long _portGoneAtTicks = -1`) and publish via `Interlocked.Exchange`/`Volatile.Write`, then reconstruct `DateTime` in the property.
- Alternatively, set the timestamp once using `Interlocked.CompareExchange` to avoid multiple writes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Previous review results

Review updated until commit 5dd7d7d ⚖️ Balanced

Results up to commit 5b0e516 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Probe errors trigger false loss ✓ Resolved 🐞 Bug ≡ Correctness

Description

SerialStreamTransport.IsPortEnumerated() swallows SerialPort.GetPortNames()/File.Exists() exceptions
and returns false, so two transient probe failures are treated as two consecutive “misses” and can
incorrectly escalate to a lost connection.
This can close a healthy serial connection and raise StatusChanged(false, ...) even though the probe
never actually observed the port disappearing.

Code

src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[R398-427]

+        try
+        {
+            foreach (var name in SerialPort.GetPortNames())
+            {
+                if (string.Equals(name, portName, StringComparison.OrdinalIgnoreCase))
+                {
+                    return true;
+                }
+            }
+        }
+        catch (Exception)
+        {
+            // Enumeration can fail transiently (a /dev scan racing a device change, a registry
+            // read on Windows). Fall through to the filesystem check rather than concluding the
+            // port has gone away.
+        }
+
+        try
+        {
+            if (portName.StartsWith('/'))
+            {
+                return File.Exists(portName);
+            }
+        }
+        catch (Exception)
+        {
+            // Same reasoning: an unreadable path is not evidence the device was unplugged.
+        }
+
+        return false;

Relevance

●●● Strong

Team previously accepted transport-hardening to avoid misleading transient failures and preserve
exceptions (PR #240, #237).

PR-#240
PR-#237

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

StartDropDetection arms presence polling using IsPortPresent(), which calls IsPortEnumerated().
IsPortEnumerated() catches enumeration/filesystem exceptions and then returns false, and
PollPresence() treats a false probe result as a miss and signals loss after two misses
(PresenceMissThreshold=2). Because the exceptions are swallowed, the watchdog’s “probe throws => no
evidence” safeguard never applies.

src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[353-384]
src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[386-428]
src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[44-55]
src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[221-254]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SerialStreamTransport.IsPortEnumerated()` currently catches exceptions from `SerialPort.GetPortNames()` and from the Unix filesystem fallback, but then falls through to `return false;`. This converts “probe failed” into “port absent”, which allows `TransportConnectionWatchdog` presence polling to count those as real misses and declare the connection lost.

### Issue Context
- Presence polling declares loss after `PresenceMissThreshold` consecutive misses.
- `TransportConnectionWatchdog.PollPresence()` intentionally treats a *throwing* probe as “no evidence” (it catches and does nothing), but `IsPortEnumerated()` prevents that by swallowing exceptions and returning `false`.

### Fix Focus Areas
- src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[386-428]
- src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs[353-375]
- src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[221-254]

### Suggested fix
Implement “inconclusive” semantics for probe failures so they do not count as absence:
- Option A (minimal): in `IsPortEnumerated`, if `SerialPort.GetPortNames()` throws, return `true` (treat as present/unknown) so polling does not increment misses. Similarly, if the Unix `File.Exists` check throws, return `true`.
- Option B (cleaner): allow exceptions to propagate out of `IsPortEnumerated` (remove the catches) and wrap the *connect-time* `IsPortPresent()` call inside `StartDropDetection()` with a try/catch that simply disables polling if the probe can’t run. This lets `TransportConnectionWatchdog.PollPresence()`’s existing `catch` correctly treat probe failures as “no evidence” (no miss increment).

Add/adjust a unit test around the presence probe path to ensure repeated probe failures do not cause a loss (e.g., by exercising `PortPresenceProbe` throwing, or by refactoring `IsPortEnumerated` to make the exception path injectable/testable).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit d5f2c6b ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Misses survive probe exception ✓ Resolved 🐞 Bug ≡ Correctness

Description

TransportConnectionWatchdog.PollPresence() keeps the current presence-miss run when the probe
throws, so a miss → exception → miss sequence can incorrectly satisfy the “consecutive misses”
threshold and signal a lost connection. This violates the intended semantics that probe exceptions
are “no observation” and should not contribute to (or preserve) an absence run.

Code

src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[R255-261]

+        catch
+        {
+            // A probe that throws is a failure to observe, not evidence of a drop: never let it
+            // disconnect a healthy transport, and never let it kill the timer thread. Probes are
+            // required to surface "could not answer" this way rather than returning false, which
+            // would be indistinguishable from the device having actually gone.
+        }

Relevance

●●● Strong

Repo strongly favors “probe exceptions aren’t evidence” semantics and robustness (PR #384
timeout/idle treated benignly); likely reset miss run.

PR-#384

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PollPresence() increments _presenceMisses on probe() == false and only resets it on `probe()
== true; when probe()` throws, the catch block is empty, so the prior miss count persists across
the exception and can incorrectly reach PresenceMissThreshold on the next observed miss.

src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[221-266]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`TransportConnectionWatchdog.PollPresence()` treats a probe exception as “failure to observe” (by swallowing it), but it does not clear `_presenceMisses`. This allows `_presenceMisses` to carry across an exception, so two `false` results separated by an exception can still be counted as “consecutive misses” and trigger a false loss.

### Issue Context
Presence polling is meant to require *consecutive observed absences*. An exception is explicitly not an observed absence, so it should break the miss run.

### Fix Focus Areas
- src/Daqifi.Core/Communication/Transport/TransportConnectionWatchdog.cs[221-266]

### Suggested fix
- In the `catch` of `PollPresence()`, clear the miss run (e.g., `Volatile.Write(ref _presenceMisses, 0);`).
- Add/extend a regression test to cover the specific sequence: `probe returns false` → `probe throws` → `probe returns false`, and assert **no** loss is signaled (i.e., the exception breaks the consecutive-miss run).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unsynchronized timestamp publication ✓ Resolved 🐞 Bug ☼ Reliability

Description

PortDisappearanceWatcher writes _portGoneAtUtc from a background thread and exposes it without
synchronization, so the main test can observe a stale/null value and log an incorrect disappearance
time. This can mislead the printed latency measurement (and the branch that chooses between “port
disappeared” vs “never disappeared”).

Code

src/Daqifi.Core.Tests/Communication/Transport/SerialUnplugValidationTests.cs[R263-274]

+                    try
+                    {
+                        if (!SerialStreamTransport.IsPortEnumerated(portName))
+                        {
+                            _portGoneAtUtc = DateTime.UtcNow;
+                            return;
+                        }
+                    }
+                    catch (Exception)
+                    {
+                        // The probe could not answer this time; that is not the port going away.
+                        // Keep watching rather than recording a bogus disappearance time.

Relevance

●● Moderate

Team accepts race/test-hardening changes (PR #237 deterministic tests; PR #240 concurrent disconnect
hardening), but no exact pattern found.

PR-#237
PR-#240

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The watcher thread sets _portGoneAtUtc when the port is first observed absent, while
PortGoneAtUtc returns the field directly with no synchronization, creating a cross-thread
publication race.

src/Daqifi.Core.Tests/Communication/Transport/SerialUnplugValidationTests.cs[251-295]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`PortDisappearanceWatcher` updates `_portGoneAtUtc` on a dedicated thread, but the value is read from the main test thread without any lock/Volatile/Interlocked publication. This is a data race and can produce stale/null reads in the diagnostic output.

### Issue Context
This is test-only code, but it is intended to produce trustworthy latency measurements and messaging.

### Fix Focus Areas
- src/Daqifi.Core.Tests/Communication/Transport/SerialUnplugValidationTests.cs[251-295]

### Suggested fix
Use one of:
- A private lock guarding both write and read of `_portGoneAtUtc`.
- Store ticks in a `long` field (e.g., `long _portGoneAtTicks = -1`) and publish via `Interlocked.Exchange`/`Volatile.Write`, then reconstruct `DateTime` in the property.
- Alternatively, set the timestamp once using `Interlocked.CompareExchange` to avoid multiple writes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[tylerkron]

Finding 1 — "Probe errors trigger false loss": VALID, fixed in d5f2c6b.

Correct, and it defeated the safeguard I thought I had. TransportConnectionWatchdog.PollPresence deliberately catches a throwing probe and treats it as no evidence — but the default probe swallowed its own exceptions and returned false, so "could not observe" arrived at the watchdog as "port is absent". Two transient enumeration failures would have counted as two consecutive misses and closed a healthy connection, which is exactly the false positive this PR is supposed to make impossible.

Took the shape of your Option B, with one refinement. IsPortEnumerated now only swallows an enumeration failure when the second source can still answer for that port name — i.e. a Unix device-node path, where File.Exists is an independent observation. For a Windows-style name there is no second source, so the exception propagates as "no observation" and the watchdog's existing guard does its job. Failure-to-observe and absence are now distinct, which is the actual invariant.

Callers updated to match:

• StartDropDetection catches a throwing baseline probe and leaves presence polling off rather than guessing — no baseline means nothing to compare later polls against. It also must not let that throw escape a connect that genuinely succeeded. I/O fault escalation still covers the port either way.
• The unplug harness's port watcher treats a throwing probe as "keep watching" instead of recording a bogus disappearance time.

Two regression tests, per your suggestion: a probe that starts throwing after arming reports no drop and stays armed, and a probe that throws at connect time leaves the check unarmed while fault escalation still detects the drop.

Full suite green on net9.0 and net10.0 (1975 Core + 23 MCP), 0 warnings.

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit d5f2c6b

[tylerkron]

Both new findings: VALID, fixed in 5dd7d7d.

1 — "Misses survive probe exception". Right, and it's the same class of mistake as the one I just fixed one layer down. I made the exception stop incrementing the run but never stop preserving it, so absent → could-not-observe → absent satisfied a two-miss threshold that never once saw the port absent twice in a row. The threshold is about consecutive observed absences, and an unobserved poll breaks consecutiveness by definition. The catch now clears the run. Regression test drives exactly the sequence you named (false → throw → false, repeated 20x) and asserts no loss and still armed.

3 — "Unsynchronized timestamp publication". Valid. Test-only, but that watcher exists for one reason — to produce a trustworthy detection-latency number for the hardware validation — and a stale read would corrupt precisely that, including the "port disappeared" vs "never disappeared" branch that tells the operator whether the cable actually came out. Took your long ticks option with Interlocked.CompareExchange on the write and Interlocked.Read on the property, which has the bonus of making "record the first absence, once" explicit rather than incidental.

Full suite green on net9.0 and net10.0 (1976 Core + 23 MCP), 0 warnings.

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 5dd7d7d