[tylerkron]

Not merging — for review.

closes #90

What was wrong

Unplug a connected DAQiFi device on macOS and the app never noticed. It kept the device in the connected list, kept showing it as connected, and left the user to work out that the only way back was to press Disconnect on a device that was already gone.

The cause: ConnectionManager's constructor built a WMI ManagementEventWatcher directly. WMI is Windows-only, so on macOS the constructor threw PlatformNotSupportedException into a catch that only wrote a log line. Hotplug-removal detection was dead from the first moment of every launch, and nothing said so. The same throw fires on iOS and Android — on Android the Sentry SDK recorded it as a Fatal breadcrumb, so it headed the breadcrumb trail of every crash report before the user had touched anything.

Reproduced on this Mac verbatim, from the removed two lines:

PlatformNotSupportedException: System.Management currently is only supported for Windows desktop applications.

What was fixed

Services/DeviceWatcher/ already had the abstraction for this — IDeviceWatcher, WmiDeviceWatcher, NoOpDeviceWatcher, DeviceWatcherFactory — but nothing was ever routed through it; ConnectionManager still constructed WMI directly. That is now wired up, and the missing POSIX-desktop backend is added.

I chose the "give macOS a working hotplug path" option, not the "make the degradation visible" fallback, because on macOS the trigger is cheap and exact: SerialPort.GetPortNames() is already the list CheckIfSerialDeviceWasRemoved compares against, so a watcher that samples it can never report a removal the handler then fails to see. There was no reason to settle for telling the user it doesn't work.

• SerialPortPollingDeviceWatcher (new) — samples SerialPort.GetPortNames() once a second and raises DeviceRemoved when the port set shrinks. Only shrinkage fires it: arrivals belong to discovery, and the Windows backend likewise subscribes to removal (EventType = 3) only. An enumeration failure returns null and leaves the previous snapshot in place, so a transient /dev problem can never look like every device being unplugged at once; those failures are logged once per outage, not once per second. Exceptions cannot escape the timer callback (that would terminate the process mid-acquisition), and each callback carries the Start/Stop generation it belongs to, so a tick still enumerating across a restart cannot compare its stale reading against the new baseline.
• DeviceWatcherFactory — WMI on Windows, the poller on macOS/Linux, NoOpDeviceWatcher on the mobile heads, which have no serial transport to watch. The startup exception is gone on all three affected heads, and with it the Fatal breadcrumb on Android.
• ConnectionManager — the catch stays, and stays load-bearing: this runs from a static field initializer, so an escaping throw would be a fatal TypeInitializationException at first access of Instance. Backend selection now happens inside it for the same reason, and a backend whose Start() throws is disposed rather than retained (WmiDeviceWatcher hooks its event before the call that can throw). What changed is that it no longer swallows. It logs which backend was chosen (USB hotplug detection backend: … — the first thing to check on a "unplugging did nothing" report), and a watcher that fails to start on a platform that has USB serial sets HotplugDetectionUnavailableMessage.
• DaqifiViewModel — turns that message into a standing notification, so the residual failure case is visible in the UI instead of log-only. It says what the user will see ("the app will keep showing it as connected until you disconnect it yourself"), not which watcher failed. Nothing is raised on the mobile heads: no serial hardware means nothing to detect, so their no-op backend is by design, not a degradation.

One pre-existing bug came along, because this PR is what makes it reachable. CheckIfSerialDeviceWasRemoved ran NotifyConnection = false on the watcher's thread, and that notification runs DaqifiViewModel.UpdateUi synchronously, which mutates the UI-bound notification collection. Off the UI thread it can throw straight back into the watcher — losing the removal before the worker that performs it is even started. It was already latent on Windows (WMI's EventArrived is a thread-pool thread too), but until now it was unreachable on macOS/Linux because the watcher never ran there. The reset now goes through a shutdown-tolerant UI post (same shape as BootloaderWatcher's existing marshal), and ConnectedDevices — a plain List mutated by Connect/Disconnect — is snapshotted on the UI thread instead of enumerated from the worker. Port enumeration, the slow part, stays off-thread.

Removing the WMI call sites also drops the five CA1416 platform-compatibility warnings they carried (desktop 297 → 292). The analyzer had been pointing at this bug the whole time.

Guarding tests

There were none, and there is no test project in this repo — that absence is part of why this shipped. Verification was done by hand at the bench with two throwaway harnesses driving the app's real code path; the checks they encode are written out below so they can be re-run or promoted if a test project ever lands.

Verification — at the bench, macOS 15 (Darwin 25.5.0), Nq1 on /dev/cu.usbmodem1101, fw 3.7.2, S/N 9090539562006014104

Build gate (clean rebuilds, --no-incremental, re-measured against current main after #149 landed — both heads built the same way in the same worktree, no remembered numbers): desktop 292 warnings vs main's 297, iOS Release iossimulator-arm64 1248 vs main's 1253. −5 on each head: the five CA1416 warnings the removed WMI call sites carried. No packages.lock.json changed. The Android head does not build on this Mac (workload absent) — CI covers it and is green; I did not run it locally.

The real app, launched on macOS. The log line at the exact call site that used to carry the exception now reads:

LEVEL=INFO: USB hotplug detection backend: SerialPortPollingDeviceWatcher
 (ConnectionManager..cctor => ConnectionManager..ctor => AppLogger.Information)

No PlatformNotSupportedException, no errors or warnings anywhere in the log. Idle at ~1% CPU over a multi-minute soak with the 1 Hz poll running; the log did not grow.

Watcher behaviour, driven against this Mac's real port table (8 ports; the board present as both /dev/cu.usbmodem1101 and /dev/tty.usbmodem1101):

• DeviceWatcherFactory.Create() returns SerialPortPollingDeviceWatcher here — PASS
• real watcher, real GetPortNames(), board left plugged in, ~25 polls: 0 DeviceRemoved — PASS (no false disconnects, the thing most likely to bite a user)
• the same real port strings minus the board's node — i.e. exactly what this Mac reports one poll after an unplug: exactly 1 DeviceRemoved — PASS
• port stays gone: no re-fire — PASS
• port comes back: no DeviceRemoved (discovery owns arrivals) — PASS
• GetPortNames() throwing: 0 DeviceRemoved, and polling resumes when it clears — PASS
• stopped watcher raises nothing even as the table shrinks; a restart re-baselines instead of reporting the ports it missed; a shrink after restart is still detected — PASS

ConnectionManager wiring, against the running singleton:

• ConnectionManager.Instance constructs with no TypeInitializationException — PASS
• HotplugDetectionUnavailableMessage is null on macOS (detection working, no nag) — PASS
• the singleton's _deviceWatcher is the polling backend and its poll timer is running — PASS
• DeviceRemoved has exactly one subscriber, Daqifi.Desktop.ConnectionManager.<.ctor>b__24_0 — PASS. This is the subscription the old code never reached on macOS: the WMI constructor threw before EventArrived was ever hooked up.

Real hardware still works through the app's own paths:

• Core SerialDeviceFinder, driven the way the connection dialog drives it: discovers Nq1 S/N=9090539562006014104 /dev/cu.usbmodem1101 — PASS
• SerialStreamingDevice.Connect() on that port: connected, reported S/N 9090539562006014104, fw 3.7.2, then disconnected cleanly — PASS

What I did not verify

The physical unplug itself was not performed — nobody was at the bench to pull the cable, and there is no software way to make a USB node disappear on macOS. The removal transition above is driven over the real port strings this Mac reports, which is the exact input the poller would see one tick after a real unplug, but the physical event was not reproduced. Worth knowing if you do reproduce it: a physical unplug also power-cycles the bus-powered device, which closing a port in software does not.

Consequently CheckIfSerialDeviceWasRemoved itself was never driven end to end against a live Avalonia dispatcher — the watcher side of the wiring is verified (correct backend, running, correctly subscribed), and the handler's UI marshalling is reasoned and build-verified, but nothing at the bench actually pulled a connected device out from under it.

Windows (WmiDeviceWatcher) and Linux were not exercised. The Windows backend is the upstream code moved behind the interface, unchanged; Linux takes the same poller as macOS. Android and iOS were not run — they take NoOpDeviceWatcher, and the claim there is only that the constructor no longer throws, which follows from the platform branch.

Re-validated after main moved

main gained #149 (DaqifiAppVersion single-sourcing in Directory.Build.props, the new AppVersion type, and the Sentry release change) after this branch was cut. Current main is merged in here rather than left to GitHub's test-merge, and everything above was re-run on the merged tree — no file overlap, no conflicts, and the warning counts in the build gate are measured against that same current main.

Re-confirmed on the merged head, at the bench with the board attached (non-destructive only — no flash, no SD write, no reboot):

• DeviceWatcherFactory.Create() returns SerialPortPollingDeviceWatcher — PASS
• 31 real SerialPort.GetPortNames() enumerations with the board attached: 0 DeviceRemoved — PASS
• the real 8-port table minus the board's two nodes: exactly 1 DeviceRemoved, no re-fire while it stays gone — PASS
• ConnectionManager.Instance constructs; HotplugDetectionUnavailableMessage is null; _deviceWatcher is the poller with a live timer; DeviceRemoved has exactly one subscriber, Daqifi.Desktop.ConnectionManager.<.ctor>b__24_0 — PASS
• the app log written by that real singleton construction reads LEVEL=INFO: USB hotplug detection backend: SerialPortPollingDeviceWatcher (ConnectionManager..cctor => ConnectionManager..ctor => AppLogger.Information) — the same call site that used to carry the PlatformNotSupportedException, with no error anywhere in the log — PASS

That log check was made headlessly against the real ConnectionManager static-constructor path, not by launching the window (the re-validation ran without a window-server session, where Avalonia's macOS render timer cannot start). It is the same call site and the same logger, but the GUI itself was not re-launched on the merged head.

Also updated

docs/RUNBOOK-macos-ios.md §1.3 documented this as an open "guarded, but degrades silently" finding with a decision still to make. Rewritten to describe the fix and what to expect at startup on each head.

🤖 Generated with Claude Code

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Fix cross-platform USB hotplug removal detection

🐞 Bug fix ✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

AI Description

• Route removal detection through platform-specific watchers instead of constructing Windows-only
 WMI.
• Poll serial ports on macOS/Linux and safely detect disappearing devices.
• Notify desktop users when hotplug monitoring cannot start.

Diagram

graph TD
  CM["Connection Manager"] --> F{"Runtime platform"} -->|Windows| W["WMI watcher"] --> R["Removal handler"] --> S["Connected state"]
  F -->|macOS / Linux| P["Port poller"] --> R
  F -->|Mobile| N["No-op watcher"]
  CM -->|Startup failure| U["User notification"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Native macOS and Linux event APIs

• ➕ Avoids periodic polling
• ➕ Can report removals with lower latency
• ➖ Requires separate IOKit and udev implementations
• ➖ Adds native interop and packaging complexity
• ➖ May diverge from the serial-port source used by removal validation

Recommendation: Keep the shared serial-port polling backend. Its one-second latency is appropriate for UI disconnect detection, it uses the same port inventory as the downstream removal check, and its guarded snapshots avoid false removals during transient enumeration failures. Native event integrations are only justified if polling cost or latency becomes measurable.

Files changed (5) +359 / -32

Enhancement (1) +38 / -7

DaqifiViewModel.csSurface unavailable hotplug detection to users +38/-7

Surface unavailable hotplug detection to users

• Adds a persistent app-level notification when USB removal monitoring cannot start on a serial-capable platform. Generalizes notification cleanup documentation to cover multiple notifications without owning devices.

Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs

Bug fix (3) +306 / -12

ConnectionManager.csRoute hotplug monitoring through the watcher abstraction +54/-9

Route hotplug monitoring through the watcher abstraction

• Replaces direct WMI construction with the platform-selected IDeviceWatcher and keeps it alive for the singleton lifetime. Logs the selected backend and records an actionable fallback message when monitoring fails on USB-capable desktop platforms.

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs

DeviceWatcherFactory.csSelect watcher backends by runtime platform +33/-3

Select watcher backends by runtime platform

• Chooses WMI on Windows, serial-port polling on macOS/Linux, and an inert watcher on mobile. Exposes whether the current platform supports serial hotplug so genuine monitoring degradation can be surfaced.

Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/DeviceWatcherFactory.cs

SerialPortPollingDeviceWatcher.csAdd resilient POSIX serial-port removal polling +219/-0

Add resilient POSIX serial-port removal polling

• Adds a one-second snapshot poller that raises DeviceRemoved only when known ports disappear. It prevents overlapping polls, preserves snapshots across enumeration failures, suppresses repeated outage warnings, and contains timer callback exceptions.

Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs

Documentation (1) +15 / -13

RUNBOOK-macos-ios.mdDocument the repaired cross-platform hotplug path +15/-13

Document the repaired cross-platform hotplug path

• Replaces the known macOS WMI limitation with the new backend matrix, polling behavior, expected startup behavior, and visible fallback notification.

docs/RUNBOOK-macos-ios.md

[qodo-code-review]

Code Review by Qodo

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

1. Hotplug callback updates UI off-thread ✓ Resolved 🐞 Bug ≡ Correctness

Description

The polling timer invokes CheckIfSerialDeviceWasRemoved on a thread-pool thread, where
NotifyConnection = false synchronously calls DaqifiViewModel.UpdateUi; that handler mutates
UI-bound collections and notification state without dispatching. A macOS/Linux unplug can therefore
trigger cross-thread UI access or lose the removal when the resulting subscriber exception is caught
by Poll.

Code

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[187]

+            watcher.DeviceRemoved += (sender, eventArgs) => CheckIfSerialDeviceWasRemoved();

Relevance

●●● Strong

Recent PR #4 accepted the same off-thread property notification and required UI-thread marshalling.

PR-#4

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new poller raises its event directly from System.Threading.Timer; the added subscription
directly enters a method that sets an observable property before any dispatcher hop. The view model
subscribes that property source to UpdateUi, whose unconditional tail and several switch branches
mutate UI-facing state, while the only existing dispatcher call occurs later inside the background
worker.

Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[146-178]
Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[531-560]
Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[693-697]
Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1708-1727]
Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1791-1803]
PR-#4

Agent prompt

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

## Issue description
The polling watcher raises `DeviceRemoved` on a timer thread, but `CheckIfSerialDeviceWasRemoved` immediately raises `NotifyConnection`, synchronously entering UI-bound `DaqifiViewModel.UpdateUi` off the Avalonia UI thread.

## Issue Context
Ensure observable property notifications and accesses to the mutable connected-device/UI collections occur on the UI thread. Keep serial-port enumeration off-thread if desired, then dispatch the state comparison and mutations safely.

## Fix Focus Areas
- Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[186-188]
- Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[531-589]
- Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1708-1803]

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

---

2. Dispatcher shutdown escapes callback ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

CheckIfSerialDeviceWasRemoved calls Dispatcher.UIThread.Post without guarding
dispatcher-shutdown failures, so a late USB removal can throw out of the WMI callback (and abort
removal processing before the worker is started). The polling backend catches this only at its outer
timer boundary, while the Windows WMI backend invokes subscribers directly.

Code

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[568]

+        Dispatcher.UIThread.Post(() => NotifyConnection = false);

Relevance

●●● Strong

Recent accepted precedents specifically require dispatcher posts to tolerate shutdown, matching this
watcher-callback reliability failure.

PR-#2
PR-#4

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed method explicitly runs on watcher threads and performs the unguarded post before
creating the BackgroundWorker; the Windows watcher directly invokes the subscriber with no
protective catch. The accepted prior review documents the same dispatcher-shutdown failure pattern
for off-thread UI posts.

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[558-570]
Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/WmiDeviceWatcher.cs[49-50]
Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[201-211]
PR-#4

Agent prompt

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

## Issue description
A late watcher callback can reach `Dispatcher.UIThread.Post` after the UI dispatcher has begun shutting down. If posting throws, the exception escapes the WMI event callback and the method never starts its reconciliation worker.

## Issue Context
Other transport-thread UI dispatch helpers in this codebase have already required shutdown-safe posting. Keep normal UI-thread marshalling, but suppress the expected dispatcher-unavailable/shutdown failure so watcher callbacks cannot propagate it.

## Fix Focus Areas
- Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[560-568]

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

---

3. Failed watcher remains retained ✓ Resolved 🐞 Bug ☼ Reliability

Description

When Start() throws after factory creation, the catch assigns the failed watcher itself to
_deviceWatcher instead of disposing it and installing the promised no-op fallback. On Windows,
WmiDeviceWatcher.Start stores a ManagementEventWatcher and subscribes its event before the
throwing Start call, so the singleton retains partially initialized resources and a live
subscription for the process lifetime.

Code

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[R198-200]

+            // A backend that could not be built or started is replaced by the one that is
+            // guaranteed inert, so every later use of the field is unconditional.
+            _deviceWatcher = watcher ?? new NoOpDeviceWatcher();

Relevance

●●● Strong

The catch retains a partially started resource; recent accepted teardown hardening supports fixing
this deterministic lifecycle leak.

PR-#125

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The catch's null-coalescing assignment chooses the non-null failed object. The WMI implementation
proves that Start() can leave state requiring Dispose(): it assigns the management watcher and
event handler before invoking the operation that may throw.

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[183-210]
Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/WmiDeviceWatcher.cs[19-46]

Agent prompt

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

## Issue description
The constructor catch retains any non-null backend whose `Start()` failed, despite documenting that the failure path installs an inert watcher.

## Issue Context
Best-effort stop/dispose the partially initialized watcher without allowing cleanup failures to escape the singleton constructor, then always assign a fresh `NoOpDeviceWatcher`.

## Fix Focus Areas
- Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[183-210]
- Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/WmiDeviceWatcher.cs[19-46]

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

---

4. Restart accepts stale polls ✓ Resolved 🐞 Bug ≡ Correctness

Description

An old timer callback that is enumerating while Stop() and Start() run can see the new non-null
_timer, compare its stale result against the new baseline, overwrite _knownPorts, and emit a
false removal. The poll has no timer-generation identity, so the _timer == null check cannot
distinguish the callback that fired before restart from callbacks belonging to the replacement
timer.

Code

Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[R166-173]

+                // Stopped or disposed between the tick firing and this point.
+                if (_timer == null)
+                {
+                    return;
+                }
+
+                anyRemoved = _knownPorts != null && !_knownPorts.IsSubsetOf(current);
+                _knownPorts = current;

Relevance

●● Moderate

The stale-restart race is plausible, but similar synchronization-race feedback was previously
rejected.

PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Stop clears the current timer and snapshot, while Start immediately permits a replacement timer
and baseline. A callback already past the reentrancy gate only checks whether some timer is non-null
after enumeration, so after restart it treats the replacement timer as proof that the old callback
is current.

Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[98-126]
Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[146-178]
Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/IDeviceWatcher.cs[12-23]

Agent prompt

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

## Issue description
Callbacks from a disposed timer can mutate state belonging to a later `Start()` generation and raise false removal events.

## Issue Context
Associate each timer/callback with a generation token and reject callbacks whose generation is no longer current, or make `Stop()` wait for in-flight callbacks before allowing restart. Also ensure `Stop()`/`Dispose()` cannot return while a validated callback can still emit an event.

## Fix Focus Areas
- Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[98-126]
- Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[146-190]

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

---

Context sources

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 341bca1

Results up to commit 7b95e2f ⚖️ Balanced

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

1. Hotplug callback updates UI off-thread ✓ Resolved 🐞 Bug ≡ Correctness

Description

The polling timer invokes CheckIfSerialDeviceWasRemoved on a thread-pool thread, where
NotifyConnection = false synchronously calls DaqifiViewModel.UpdateUi; that handler mutates
UI-bound collections and notification state without dispatching. A macOS/Linux unplug can therefore
trigger cross-thread UI access or lose the removal when the resulting subscriber exception is caught
by Poll.

Code

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[187]

+            watcher.DeviceRemoved += (sender, eventArgs) => CheckIfSerialDeviceWasRemoved();

Relevance

●●● Strong

Recent PR #4 accepted the same off-thread property notification and required UI-thread marshalling.

PR-#4

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new poller raises its event directly from System.Threading.Timer; the added subscription
directly enters a method that sets an observable property before any dispatcher hop. The view model
subscribes that property source to UpdateUi, whose unconditional tail and several switch branches
mutate UI-facing state, while the only existing dispatcher call occurs later inside the background
worker.

Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[146-178]
Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[531-560]
Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[693-697]
Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1708-1727]
Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1791-1803]
PR-#4

Agent prompt

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

## Issue description
The polling watcher raises `DeviceRemoved` on a timer thread, but `CheckIfSerialDeviceWasRemoved` immediately raises `NotifyConnection`, synchronously entering UI-bound `DaqifiViewModel.UpdateUi` off the Avalonia UI thread.

## Issue Context
Ensure observable property notifications and accesses to the mutable connected-device/UI collections occur on the UI thread. Keep serial-port enumeration off-thread if desired, then dispatch the state comparison and mutations safely.

## Fix Focus Areas
- Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[186-188]
- Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[531-589]
- Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[1708-1803]

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

---

2. Failed watcher remains retained ✓ Resolved 🐞 Bug ☼ Reliability

Description

When Start() throws after factory creation, the catch assigns the failed watcher itself to
_deviceWatcher instead of disposing it and installing the promised no-op fallback. On Windows,
WmiDeviceWatcher.Start stores a ManagementEventWatcher and subscribes its event before the
throwing Start call, so the singleton retains partially initialized resources and a live
subscription for the process lifetime.

Code

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[R198-200]

+            // A backend that could not be built or started is replaced by the one that is
+            // guaranteed inert, so every later use of the field is unconditional.
+            _deviceWatcher = watcher ?? new NoOpDeviceWatcher();

Relevance

●●● Strong

The catch retains a partially started resource; recent accepted teardown hardening supports fixing
this deterministic lifecycle leak.

PR-#125

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The catch's null-coalescing assignment chooses the non-null failed object. The WMI implementation
proves that Start() can leave state requiring Dispose(): it assigns the management watcher and
event handler before invoking the operation that may throw.

Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[183-210]
Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/WmiDeviceWatcher.cs[19-46]

Agent prompt

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

## Issue description
The constructor catch retains any non-null backend whose `Start()` failed, despite documenting that the failure path installs an inert watcher.

## Issue Context
Best-effort stop/dispose the partially initialized watcher without allowing cleanup failures to escape the singleton constructor, then always assign a fresh `NoOpDeviceWatcher`.

## Fix Focus Areas
- Daqifi.Avalonia/Daqifi.Desktop/ConnectionManager.cs[183-210]
- Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/WmiDeviceWatcher.cs[19-46]

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

---

3. Restart accepts stale polls ✓ Resolved 🐞 Bug ≡ Correctness

Description

An old timer callback that is enumerating while Stop() and Start() run can see the new non-null
_timer, compare its stale result against the new baseline, overwrite _knownPorts, and emit a
false removal. The poll has no timer-generation identity, so the _timer == null check cannot
distinguish the callback that fired before restart from callbacks belonging to the replacement
timer.

Code

Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[R166-173]

+                // Stopped or disposed between the tick firing and this point.
+                if (_timer == null)
+                {
+                    return;
+                }
+
+                anyRemoved = _knownPorts != null && !_knownPorts.IsSubsetOf(current);
+                _knownPorts = current;

Relevance

●● Moderate

The stale-restart race is plausible, but similar synchronization-race feedback was previously
rejected.

PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Stop clears the current timer and snapshot, while Start immediately permits a replacement timer
and baseline. A callback already past the reentrancy gate only checks whether some timer is non-null
after enumeration, so after restart it treats the replacement timer as proof that the old callback
is current.

Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[98-126]
Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[146-178]
Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/IDeviceWatcher.cs[12-23]

Agent prompt

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

## Issue description
Callbacks from a disposed timer can mutate state belonging to a later `Start()` generation and raise false removal events.

## Issue Context
Associate each timer/callback with a generation token and reject callbacks whose generation is no longer current, or make `Stop()` wait for in-flight callbacks before allowing restart. Also ensure `Stop()`/`Dispose()` cannot return while a validated callback can still emit an event.

## Fix Focus Areas
- Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[98-126]
- Daqifi.Avalonia/Daqifi.Desktop/Services/DeviceWatcher/SerialPortPollingDeviceWatcher.cs[146-190]

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

---

[tylerkron]

/agentic_review

[qodo-code-review]

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

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 19db6ab

[tylerkron]

/agentic_review

[qodo-code-review]

No code changes since the last review — review skipped

[tylerkron]

Qodo-clean, CI green — ready for review. (Re-validated on 6059ad0, current main merged in: Qodo Bugs (0) / 0 unresolved threads pinned to that head and re-checked after a settle, all six CI checks green including the Android head, and the bench re-confirmation re-run on the merged tree. The physical unplug is still not verified — see the body.)