feat(firmware): split WiFi update planning from execution (closes #143)

[cptkoolbeenz]

Summary

Adds a public CheckWifiFirmwareStatusAsync planning method and a skipVersionCheck parameter on UpdateWifiModuleAsync so callers (typically desktop UI) can make the version decision once and run the flash without triggering Core's hidden second probe.

The previous flow conflated two responsibilities — desktop's workaround was to query the version separately, then route the flash through an adapter that hid ILanChipInfoProvider so Core would skip its internal probe. The issue explicitly called this out as an API-shape workaround.

API additions

• WifiFirmwareStatus record + WifiFirmwareStatusReason enum exposing chip info, latest release, and a categorical decision so callers distinguish "definitively up to date" from "couldn't check".
• IFirmwareUpdateService.CheckWifiFirmwareStatusAsync — pure planning method (does NOT mutate service state, no Complete transition, no progress events).
• IFirmwareUpdateService.UpdateWifiModuleAsync gains skipVersionCheck: false default. Existing callers unchanged; callers that just made the decision pass true.

Implementation

Pulls data-gathering out of IsWifiFirmwareUpToDateAsync into CheckWifiFirmwareStatusCoreAsync. The legacy method delegates to the new core helper and only adds the side effects (Complete state + progress) needed by the all-in-one caller path. Core remains the source of truth for version comparison (IsWifiVersionCurrent).

Test plan

• 4 new tests cover the planning method's three primary outcomes (UpToDate / UpdateAvailable / DeviceDoesNotSupportLanQuery) and the skipVersionCheck pathway
• All 5 existing UpdateWifiModuleAsync tests still pass (no behavior change for current callers)
• All 47 WiFi-related tests pass; full suite 894/896 (2 skipped require live hardware)
• Build clean on net9.0 + net10.0

Closes #143

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

✅ Tickets: 🎫 Separate WiFi update planning from WiFi flash execution

1. Planning bypasses operation lock ✓ Resolved 🐞 Bug ☼ Reliability

Description

CheckWifiFirmwareStatusAsync does not participate in FirmwareUpdateService’s _operationLock, so it
can query the device while an update is running and race with connect/disconnect/SCPI traffic. This
can yield spurious status results (e.g., ChipInfoUnavailable) or interfere with an active flash
flow.

Code

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[R182-191]

+    /// <inheritdoc />
+    public async Task<WifiFirmwareStatus> CheckWifiFirmwareStatusAsync(
+        IStreamingDevice device,
+        CancellationToken cancellationToken = default)
+    {
+        ArgumentNullException.ThrowIfNull(device);
+        ThrowIfDisposed();
+
+        return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false);
+    }

Evidence

Update operations are serialized with _operationLock inside RunExclusiveAsync, but the new
planning API bypasses it and still performs SCPI-based device queries, enabling concurrent access
and races during update-driven disconnect/reconnect phases.

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-191]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[193-219]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[842-856]

Agent prompt

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

## Issue description
`CheckWifiFirmwareStatusAsync` calls `CheckWifiFirmwareStatusCoreAsync` directly and does not acquire `_operationLock`. This allows concurrent device I/O with `UpdateWifiModuleAsync` / `UpdateFirmwareAsync`, which are explicitly serialized via `RunExclusiveAsync`.

## Issue Context
- `RunExclusiveAsync` enforces single-operation execution and performs state validation.
- LAN chip probing (`GetLanChipInfoAsync`) sends SCPI and requires the device be connected; concurrent update operations disconnect/reconnect the device.

## Fix Focus Areas
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[182-219]
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[842-856]

## Suggested fix
- Acquire `_operationLock` in `CheckWifiFirmwareStatusAsync` (or introduce a new helper like `RunExclusiveReadOnlyAsync`) to serialize planning calls with update operations, **without** mutating update state.
- Optionally, fail fast if `CurrentState != Idle` to avoid waiting on long-running updates, depending on intended UX/API contract.

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

---

2. Positional CancellationToken call break ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

UpdateWifiModuleAsync inserts skipVersionCheck before cancellationToken, so any existing
positional call that previously passed a CancellationToken as the 4th argument will no longer
compile. This is a source-breaking change in a public interface/API surface.

Code

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[R41-52]

+    /// <param name="skipVersionCheck">
+    /// When true, bypass the internal version probe and always run the flash
+    /// flow. Use after a separate <see cref="CheckWifiFirmwareStatusAsync"/>
+    /// call so the device isn't queried twice — see issue #143 for the
+    /// motivating callsite.
+    /// </param>
    Task UpdateWifiModuleAsync(
        IStreamingDevice device,
        string firmwarePath,
        IProgress<FirmwareUpdateProgress>? progress = null,
+        bool skipVersionCheck = false,
+        CancellationToken cancellationToken = default);

Evidence

The interface and implementation signatures now place skipVersionCheck before cancellationToken,
shifting the CancellationToken argument position and breaking existing positional CancellationToken
call sites.

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[34-52]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-180]

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 public API adds `bool skipVersionCheck` before `CancellationToken cancellationToken`, changing the positional index of the CancellationToken parameter. Existing consumers with calls like `UpdateWifiModuleAsync(device, path, progress, cancellationToken)` will now fail to compile.

## Issue Context
This repo has no internal call sites using a positional CancellationToken, but `IFirmwareUpdateService` is public and this change is a likely integration breaker for external consumers.

## Fix Focus Areas
- src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[34-52]
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-180]

## Suggested fix options
1) **Reorder parameters** to keep CancellationToken as the 4th parameter for backward compatibility, e.g.:
  `(..., IProgress<...>? progress = null, CancellationToken cancellationToken = default, bool skipVersionCheck = false)`
  (Tradeoff: CancellationToken is no longer last.)

2) **Introduce a new method name** (or a non-optional overload strategy that avoids ambiguity) to preserve the original signature for existing callers while offering the new capability.

Also update XML docs accordingly.

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

---

3. Deadlock test lacks timeout ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new regression test documents a failure mode of “hangs”, but it doesn’t enforce any explicit
timeout around the awaited update call. If the deadlock regresses, the test run can stall
indefinitely instead of failing fast.

Code

src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs[R653-722]

+    [Fact]
+    public async Task CheckWifiFirmwareStatusAsync_ReentrantCallFromUpdateProgressCallback_DoesNotDeadlock()
+    {
+        // Closes a Qodo finding on PR #198: UpdateWifiModuleAsync holds
+        // _operationLock while synchronously firing progress.Report().
+        // A handler that calls back into CheckWifiFirmwareStatusAsync
+        // would deadlock waiting for the same lock the update flow owns
+        // (SemaphoreSlim is not re-entrant). The AsyncLocal _isInsideOperation
+        // flag detects this case and skips the second acquisition.
+        //
+        // The test invokes CheckWifiFirmwareStatusAsync from inside a
+        // progress callback fired by UpdateWifiModuleAsync. Without the
+        // re-entrancy guard, this hangs until xunit's per-test budget
+        // kills it. With the guard, the inner call returns quickly and
+        // the update completes normally.
+        var wifiRelease = new FirmwareReleaseInfo
+        {
+            Version = new FirmwareVersion(19, 6, 1, null, 0),
+            TagName = "19.6.1",
+            IsPreRelease = false
+        };
+        var device = new FakeLanChipInfoStreamingDevice("COM13", chipInfo: new LanChipInfo
+        {
+            ChipId = 1234,
+            FwVersion = "19.5.4",
+            BuildDate = "Jan  8 2019"
+        });
+
+        var externalProcessRunner = new FakeExternalProcessRunner
+        {
+            NextResult = new ExternalProcessResult(0, false, TimeSpan.FromMilliseconds(10), [], [])
+        };
+
+        var options = CreateFastOptions();
+        options.PostLanFirmwareModeDelay = TimeSpan.FromMilliseconds(5);
+        options.PostWifiReconnectDelay = TimeSpan.FromMilliseconds(5);
+
+        var service = new FirmwareUpdateService(
+            new FakeHidTransport(),
+            new FakeFirmwareDownloadService { LatestWifiRelease = wifiRelease },
+            externalProcessRunner,
+            NullLogger<FirmwareUpdateService>.Instance,
+            new FakeBootloaderProtocol([[0x10]]),
+            new FakeHidDeviceEnumerator([]),
+            options);
+
+        var firmwareDir = CreateTempDirectory();
+        File.WriteAllText(Path.Combine(firmwareDir, "winc_flash_tool.cmd"), "@echo off");
+
+        WifiFirmwareStatus? reentrantStatus = null;
+        var reentryAttempted = false;
+
+        // Progress handler that re-enters CheckWifiFirmwareStatusAsync
+        // exactly once (on the first event) — mirrors a UI consumer
+        // that might want to refresh status display when an update
+        // transitions state.
+        var progress = new SyncProgress<FirmwareUpdateProgress>(_ =>
+        {
+            if (reentryAttempted)
+            {
+                return;
+            }
+            reentryAttempted = true;
+            reentrantStatus = service.CheckWifiFirmwareStatusAsync(device).GetAwaiter().GetResult();
+        });
+
+        try
+        {
+            await service.UpdateWifiModuleAsync(device, firmwareDir, progress);
+        }

Evidence

The test states it would “hang” without the guard, and the implementation awaits the update call
without any explicit timeout mechanism.

src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs[653-667]
src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs[719-722]

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 regression in the reentrancy guard would cause this test to hang, but the test doesn’t apply an explicit timeout. This can stall CI rather than producing a clear failing test.

### Issue Context
The test itself explains the hang scenario, but relies on unspecified runner behavior (“xunit’s per-test budget”).

### Fix Focus Areas
- src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs[653-731]

### Suggested fix
Add a hard timeout to fail fast, e.g.:
- Use xUnit timeout support if available: `[Fact(Timeout = 10000)]`.
- Or wrap with a timeout token:
 - `using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));`
 - pass `cts.Token` to `UpdateWifiModuleAsync(...)` (and/or use `Task.WhenAny(updateTask, Task.Delay(...))`).

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

---

4. Callback reentrancy deadlock ✓ Resolved 🐞 Bug ☼ Reliability

Description

CheckWifiFirmwareStatusAsync waits on _operationLock; firmware updates hold that same lock while
invoking progress.Report and StateChanged handlers synchronously. If a consumer calls
CheckWifiFirmwareStatusAsync from either callback, the callback blocks waiting for the lock while
the update cannot continue until the callback returns, causing a deadlock/hang.

Code

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[R190-205]

+        // Serialize device I/O with UpdateFirmwareAsync / UpdateWifiModuleAsync.
+        // GetLanChipInfoAsync runs a SCPI text exchange on the same transport
+        // those updates use; a concurrent caller would interleave on the wire
+        // and either corrupt the consumer swap or get partial replies.
+        // Acquired without the RunExclusiveAsync state check — a status probe
+        // is read-only and should be available to UI even when an update is
+        // in flight (it just waits for the in-flight I/O to release the lock).
+        await _operationLock.WaitAsync(cancellationToken).ConfigureAwait(false);
+        try
+        {
+            return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false);
+        }
+        finally
+        {
+            _operationLock.Release();
+        }

Evidence

The new method blocks on _operationLock. Updates acquire the same lock for the entire operation
and, while holding it, synchronously invoke callbacks; re-entering the service from those callbacks
to call the new method will block on the lock and prevent the update from progressing, forming a
deadlock.

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[182-206]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[208-233]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1148-1167]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1169-1195]

Agent prompt

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

## Issue description
`CheckWifiFirmwareStatusAsync` awaits `_operationLock`, and update operations hold `_operationLock` while synchronously invoking `progress.Report(...)` and `StateChanged?.Invoke(...)`. If a callback calls back into `CheckWifiFirmwareStatusAsync`, it will deadlock.

## Issue Context
This is a classic non-reentrant lock + synchronous callback reentrancy trap. The new public API makes it more likely for consumers (especially UI) to call the status method in response to update callbacks.

## Fix Focus Areas
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[190-205]

## Suggested fix approach
- Add explicit XML doc / remarks and/or inline comments warning that `CheckWifiFirmwareStatusAsync` must not be called from `StateChanged` handlers or progress callbacks.
- Preferably, refactor so callbacks (`progress.Report`, `StateChanged`) are invoked outside `_operationLock` (e.g., capture state/progress data under the lock, then invoke callbacks after releasing it), or introduce a non-blocking status check variant that fails fast when the lock is held to avoid deadlock scenarios.

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

---

Previous review results

Review updated until commit 2496323

Results up to commit f3a37ec

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Planning bypasses operation lock ✓ Resolved 🐞 Bug ☼ Reliability

Description

CheckWifiFirmwareStatusAsync does not participate in FirmwareUpdateService’s _operationLock, so it
can query the device while an update is running and race with connect/disconnect/SCPI traffic. This
can yield spurious status results (e.g., ChipInfoUnavailable) or interfere with an active flash
flow.

Code

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[R182-191]

+    /// <inheritdoc />
+    public async Task<WifiFirmwareStatus> CheckWifiFirmwareStatusAsync(
+        IStreamingDevice device,
+        CancellationToken cancellationToken = default)
+    {
+        ArgumentNullException.ThrowIfNull(device);
+        ThrowIfDisposed();
+
+        return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false);
+    }

Evidence

Update operations are serialized with _operationLock inside RunExclusiveAsync, but the new
planning API bypasses it and still performs SCPI-based device queries, enabling concurrent access
and races during update-driven disconnect/reconnect phases.

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-191]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[193-219]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[842-856]

Agent prompt

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

## Issue description
`CheckWifiFirmwareStatusAsync` calls `CheckWifiFirmwareStatusCoreAsync` directly and does not acquire `_operationLock`. This allows concurrent device I/O with `UpdateWifiModuleAsync` / `UpdateFirmwareAsync`, which are explicitly serialized via `RunExclusiveAsync`.

## Issue Context
- `RunExclusiveAsync` enforces single-operation execution and performs state validation.
- LAN chip probing (`GetLanChipInfoAsync`) sends SCPI and requires the device be connected; concurrent update operations disconnect/reconnect the device.

## Fix Focus Areas
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[182-219]
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[842-856]

## Suggested fix
- Acquire `_operationLock` in `CheckWifiFirmwareStatusAsync` (or introduce a new helper like `RunExclusiveReadOnlyAsync`) to serialize planning calls with update operations, **without** mutating update state.
- Optionally, fail fast if `CurrentState != Idle` to avoid waiting on long-running updates, depending on intended UX/API contract.

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

---

2. Positional CancellationToken call break ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

UpdateWifiModuleAsync inserts skipVersionCheck before cancellationToken, so any existing
positional call that previously passed a CancellationToken as the 4th argument will no longer
compile. This is a source-breaking change in a public interface/API surface.

Code

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[R41-52]

+    /// <param name="skipVersionCheck">
+    /// When true, bypass the internal version probe and always run the flash
+    /// flow. Use after a separate <see cref="CheckWifiFirmwareStatusAsync"/>
+    /// call so the device isn't queried twice — see issue #143 for the
+    /// motivating callsite.
+    /// </param>
    Task UpdateWifiModuleAsync(
        IStreamingDevice device,
        string firmwarePath,
        IProgress<FirmwareUpdateProgress>? progress = null,
+        bool skipVersionCheck = false,
+        CancellationToken cancellationToken = default);

Evidence

The interface and implementation signatures now place skipVersionCheck before cancellationToken,
shifting the CancellationToken argument position and breaking existing positional CancellationToken
call sites.

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[34-52]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-180]

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 public API adds `bool skipVersionCheck` before `CancellationToken cancellationToken`, changing the positional index of the CancellationToken parameter. Existing consumers with calls like `UpdateWifiModuleAsync(device, path, progress, cancellationToken)` will now fail to compile.

## Issue Context
This repo has no internal call sites using a positional CancellationToken, but `IFirmwareUpdateService` is public and this change is a likely integration breaker for external consumers.

## Fix Focus Areas
- src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[34-52]
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-180]

## Suggested fix options
1) **Reorder parameters** to keep CancellationToken as the 4th parameter for backward compatibility, e.g.:
  `(..., IProgress<...>? progress = null, CancellationToken cancellationToken = default, bool skipVersionCheck = false)`
  (Tradeoff: CancellationToken is no longer last.)

2) **Introduce a new method name** (or a non-optional overload strategy that avoids ambiguity) to preserve the original signature for existing callers while offering the new capability.

Also update XML docs accordingly.

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

---

Results up to commit 1ccb31b

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Callback reentrancy deadlock ✓ Resolved 🐞 Bug ☼ Reliability

Description

CheckWifiFirmwareStatusAsync waits on _operationLock; firmware updates hold that same lock while
invoking progress.Report and StateChanged handlers synchronously. If a consumer calls
CheckWifiFirmwareStatusAsync from either callback, the callback blocks waiting for the lock while
the update cannot continue until the callback returns, causing a deadlock/hang.

Code

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[R190-205]

+        // Serialize device I/O with UpdateFirmwareAsync / UpdateWifiModuleAsync.
+        // GetLanChipInfoAsync runs a SCPI text exchange on the same transport
+        // those updates use; a concurrent caller would interleave on the wire
+        // and either corrupt the consumer swap or get partial replies.
+        // Acquired without the RunExclusiveAsync state check — a status probe
+        // is read-only and should be available to UI even when an update is
+        // in flight (it just waits for the in-flight I/O to release the lock).
+        await _operationLock.WaitAsync(cancellationToken).ConfigureAwait(false);
+        try
+        {
+            return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false);
+        }
+        finally
+        {
+            _operationLock.Release();
+        }

Evidence

The new method blocks on _operationLock. Updates acquire the same lock for the entire operation
and, while holding it, synchronously invoke callbacks; re-entering the service from those callbacks
to call the new method will block on the lock and prevent the update from progressing, forming a
deadlock.

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[182-206]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[208-233]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1148-1167]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[1169-1195]

Agent prompt

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

## Issue description
`CheckWifiFirmwareStatusAsync` awaits `_operationLock`, and update operations hold `_operationLock` while synchronously invoking `progress.Report(...)` and `StateChanged?.Invoke(...)`. If a callback calls back into `CheckWifiFirmwareStatusAsync`, it will deadlock.

## Issue Context
This is a classic non-reentrant lock + synchronous callback reentrancy trap. The new public API makes it more likely for consumers (especially UI) to call the status method in response to update callbacks.

## Fix Focus Areas
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[190-205]

## Suggested fix approach
- Add explicit XML doc / remarks and/or inline comments warning that `CheckWifiFirmwareStatusAsync` must not be called from `StateChanged` handlers or progress callbacks.
- Preferably, refactor so callbacks (`progress.Report`, `StateChanged`) are invoked outside `_operationLock` (e.g., capture state/progress data under the lock, then invoke callbacks after releasing it), or introduce a non-blocking status check variant that fails fast when the lock is held to avoid deadlock scenarios.

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

---

Results up to commit b076e5f

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Deadlock test lacks timeout ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new regression test documents a failure mode of “hangs”, but it doesn’t enforce any explicit
timeout around the awaited update call. If the deadlock regresses, the test run can stall
indefinitely instead of failing fast.

Code

src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs[R653-722]

+    [Fact]
+    public async Task CheckWifiFirmwareStatusAsync_ReentrantCallFromUpdateProgressCallback_DoesNotDeadlock()
+    {
+        // Closes a Qodo finding on PR #198: UpdateWifiModuleAsync holds
+        // _operationLock while synchronously firing progress.Report().
+        // A handler that calls back into CheckWifiFirmwareStatusAsync
+        // would deadlock waiting for the same lock the update flow owns
+        // (SemaphoreSlim is not re-entrant). The AsyncLocal _isInsideOperation
+        // flag detects this case and skips the second acquisition.
+        //
+        // The test invokes CheckWifiFirmwareStatusAsync from inside a
+        // progress callback fired by UpdateWifiModuleAsync. Without the
+        // re-entrancy guard, this hangs until xunit's per-test budget
+        // kills it. With the guard, the inner call returns quickly and
+        // the update completes normally.
+        var wifiRelease = new FirmwareReleaseInfo
+        {
+            Version = new FirmwareVersion(19, 6, 1, null, 0),
+            TagName = "19.6.1",
+            IsPreRelease = false
+        };
+        var device = new FakeLanChipInfoStreamingDevice("COM13", chipInfo: new LanChipInfo
+        {
+            ChipId = 1234,
+            FwVersion = "19.5.4",
+            BuildDate = "Jan  8 2019"
+        });
+
+        var externalProcessRunner = new FakeExternalProcessRunner
+        {
+            NextResult = new ExternalProcessResult(0, false, TimeSpan.FromMilliseconds(10), [], [])
+        };
+
+        var options = CreateFastOptions();
+        options.PostLanFirmwareModeDelay = TimeSpan.FromMilliseconds(5);
+        options.PostWifiReconnectDelay = TimeSpan.FromMilliseconds(5);
+
+        var service = new FirmwareUpdateService(
+            new FakeHidTransport(),
+            new FakeFirmwareDownloadService { LatestWifiRelease = wifiRelease },
+            externalProcessRunner,
+            NullLogger<FirmwareUpdateService>.Instance,
+            new FakeBootloaderProtocol([[0x10]]),
+            new FakeHidDeviceEnumerator([]),
+            options);
+
+        var firmwareDir = CreateTempDirectory();
+        File.WriteAllText(Path.Combine(firmwareDir, "winc_flash_tool.cmd"), "@echo off");
+
+        WifiFirmwareStatus? reentrantStatus = null;
+        var reentryAttempted = false;
+
+        // Progress handler that re-enters CheckWifiFirmwareStatusAsync
+        // exactly once (on the first event) — mirrors a UI consumer
+        // that might want to refresh status display when an update
+        // transitions state.
+        var progress = new SyncProgress<FirmwareUpdateProgress>(_ =>
+        {
+            if (reentryAttempted)
+            {
+                return;
+            }
+            reentryAttempted = true;
+            reentrantStatus = service.CheckWifiFirmwareStatusAsync(device).GetAwaiter().GetResult();
+        });
+
+        try
+        {
+            await service.UpdateWifiModuleAsync(device, firmwareDir, progress);
+        }

Evidence

The test states it would “hang” without the guard, and the implementation awaits the update call
without any explicit timeout mechanism.

src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs[653-667]
src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs[719-722]

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 regression in the reentrancy guard would cause this test to hang, but the test doesn’t apply an explicit timeout. This can stall CI rather than producing a clear failing test.

### Issue Context
The test itself explains the hang scenario, but relies on unspecified runner behavior (“xunit’s per-test budget”).

### Fix Focus Areas
- src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs[653-731]

### Suggested fix
Add a hard timeout to fail fast, e.g.:
- Use xUnit timeout support if available: `[Fact(Timeout = 10000)]`.
- Or wrap with a timeout token:
 - `using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));`
 - pass `cts.Token` to `UpdateWifiModuleAsync(...)` (and/or use `Task.WhenAny(updateTask, Task.Delay(...))`).

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

---

[qodo-code-review]

Review Summary by Qodo

Split WiFi firmware update planning from execution

✨ Enhancement

Walkthroughs

Description

• Add public CheckWifiFirmwareStatusAsync planning method for WiFi firmware status inspection
• Introduce WifiFirmwareStatus record and WifiFirmwareStatusReason enum for detailed decision
  reporting
• Add skipVersionCheck parameter to UpdateWifiModuleAsync to bypass internal version probe
• Refactor version-checking logic into reusable CheckWifiFirmwareStatusCoreAsync helper
• Add 4 new tests covering planning method outcomes and skip-check pathway

Diagram

flowchart LR
  A["Caller"] -->|"CheckWifiFirmwareStatusAsync"| B["CheckWifiFirmwareStatusCoreAsync"]
  B -->|"returns WifiFirmwareStatus"| C["Caller decides"]
  C -->|"UpdateWifiModuleAsync<br/>skipVersionCheck=true"| D["RunWifiUpdateAsync"]
  D -->|"skips probe"| E["Flash device"]
  E -->|"Complete"| F["Done"]

Loading

File Changes

1. src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs ✨ Enhancement +67/-0

New WiFi firmware status types

• New file introducing WifiFirmwareStatus record with CurrentChipInfo, LatestRelease,
 IsUpToDate, and Reason properties
• New WifiFirmwareStatusReason enum with six categorical outcomes: UpToDate, UpdateAvailable,
 DeviceDoesNotSupportLanQuery, ChipInfoUnavailable, LatestReleaseUnavailable,
 VersionUnparseable
• Comprehensive XML documentation explaining the status record's role in separating planning from
 execution

src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs

---

2. src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs ✨ Enhancement +21/-0

Add planning method and skip-check parameter

• Add skipVersionCheck parameter (default false) to UpdateWifiModuleAsync signature
• Add new CheckWifiFirmwareStatusAsync method returning Task<WifiFirmwareStatus>
• Update XML documentation for both methods explaining the planning/execution split and #143
 motivation

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs

---

3. src/Daqifi.Core/Firmware/FirmwareUpdateService.cs ✨ Enhancement +98/-20

Refactor version checking and add planning method

• Implement public CheckWifiFirmwareStatusAsync method delegating to new
 CheckWifiFirmwareStatusCoreAsync
• Extract version-checking logic from IsWifiFirmwareUpToDateAsync into
 CheckWifiFirmwareStatusCoreAsync returning WifiFirmwareStatus
• Refactor IsWifiFirmwareUpToDateAsync to delegate to core helper and add side effects (state
 transition, progress reporting)
• Update RunWifiUpdateAsync to accept skipVersionCheck parameter and conditionally skip version
 probe
• Replace boolean returns with WifiFirmwareStatus objects throughout, handling all six reason
 codes

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs

---

View more (1)

4. src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs 🧪 Tests +159/-0

Add four new tests for planning method

• Add test CheckWifiFirmwareStatusAsync_WhenVersionMatches_ReturnsUpToDate verifying planning
 method returns UpToDate status without mutating service state
• Add test CheckWifiFirmwareStatusAsync_WhenVersionOlder_ReturnsUpdateAvailable verifying
 UpdateAvailable reason
• Add test
 CheckWifiFirmwareStatusAsync_WhenDeviceDoesNotSupportLanQuery_ReturnsDeviceDoesNotSupportLanQuery
 verifying unsupported device handling
• Add test UpdateWifiModuleAsync_WithSkipVersionCheck_BypassesProbeAndAlwaysFlashes verifying
 skip-check bypasses version probe even when versions match

src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Positional CancellationToken call break ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

UpdateWifiModuleAsync inserts skipVersionCheck before cancellationToken, so any existing
positional call that previously passed a CancellationToken as the 4th argument will no longer
compile. This is a source-breaking change in a public interface/API surface.

Code

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[R41-52]

+    /// <param name="skipVersionCheck">
+    /// When true, bypass the internal version probe and always run the flash
+    /// flow. Use after a separate <see cref="CheckWifiFirmwareStatusAsync"/>
+    /// call so the device isn't queried twice — see issue #143 for the
+    /// motivating callsite.
+    /// </param>
    Task UpdateWifiModuleAsync(
        IStreamingDevice device,
        string firmwarePath,
        IProgress<FirmwareUpdateProgress>? progress = null,
+        bool skipVersionCheck = false,
+        CancellationToken cancellationToken = default);

Evidence

The interface and implementation signatures now place skipVersionCheck before cancellationToken,
shifting the CancellationToken argument position and breaking existing positional CancellationToken
call sites.

src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[34-52]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-180]

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 public API adds `bool skipVersionCheck` before `CancellationToken cancellationToken`, changing the positional index of the CancellationToken parameter. Existing consumers with calls like `UpdateWifiModuleAsync(device, path, progress, cancellationToken)` will now fail to compile.

## Issue Context
This repo has no internal call sites using a positional CancellationToken, but `IFirmwareUpdateService` is public and this change is a likely integration breaker for external consumers.

## Fix Focus Areas
- src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs[34-52]
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-180]

## Suggested fix options
1) **Reorder parameters** to keep CancellationToken as the 4th parameter for backward compatibility, e.g.:
  `(..., IProgress<...>? progress = null, CancellationToken cancellationToken = default, bool skipVersionCheck = false)`
  (Tradeoff: CancellationToken is no longer last.)

2) **Introduce a new method name** (or a non-optional overload strategy that avoids ambiguity) to preserve the original signature for existing callers while offering the new capability.

Also update XML docs accordingly.

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

---

2. Planning bypasses operation lock ✓ Resolved 🐞 Bug ☼ Reliability

Description

CheckWifiFirmwareStatusAsync does not participate in FirmwareUpdateService’s _operationLock, so it
can query the device while an update is running and race with connect/disconnect/SCPI traffic. This
can yield spurious status results (e.g., ChipInfoUnavailable) or interfere with an active flash
flow.

Code

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[R182-191]

+    /// <inheritdoc />
+    public async Task<WifiFirmwareStatus> CheckWifiFirmwareStatusAsync(
+        IStreamingDevice device,
+        CancellationToken cancellationToken = default)
+    {
+        ArgumentNullException.ThrowIfNull(device);
+        ThrowIfDisposed();
+
+        return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false);
+    }

Evidence

Update operations are serialized with _operationLock inside RunExclusiveAsync, but the new
planning API bypasses it and still performs SCPI-based device queries, enabling concurrent access
and races during update-driven disconnect/reconnect phases.

src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[156-191]
src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[193-219]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[842-856]

Agent prompt

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

## Issue description
`CheckWifiFirmwareStatusAsync` calls `CheckWifiFirmwareStatusCoreAsync` directly and does not acquire `_operationLock`. This allows concurrent device I/O with `UpdateWifiModuleAsync` / `UpdateFirmwareAsync`, which are explicitly serialized via `RunExclusiveAsync`.

## Issue Context
- `RunExclusiveAsync` enforces single-operation execution and performs state validation.
- LAN chip probing (`GetLanChipInfoAsync`) sends SCPI and requires the device be connected; concurrent update operations disconnect/reconnect the device.

## Fix Focus Areas
- src/Daqifi.Core/Firmware/FirmwareUpdateService.cs[182-219]
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[842-856]

## Suggested fix
- Acquire `_operationLock` in `CheckWifiFirmwareStatusAsync` (or introduce a new helper like `RunExclusiveReadOnlyAsync`) to serialize planning calls with update operations, **without** mutating update state.
- Optionally, fail fast if `CurrentState != Idle` to avoid waiting on long-running updates, depending on intended UX/API contract.

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

---

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 2496323

Warning

/improve is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Category	Suggestion	Impact
Possible issue	Enforce timeout even if uncancelable Wrap the GetLanChipInfoAsync task with WaitAsync(linkedToken) to enforce the cancellation timeout. src/Daqifi.Core/Firmware/FirmwareUpdateService.cs [263-274] -var chipInfo = await lanChipInfoProvider.GetLanChipInfoAsync(linkedToken).ConfigureAwait(false); +var chipInfoTask = lanChipInfoProvider.GetLanChipInfoAsync(linkedToken); +var chipInfo = await chipInfoTask.WaitAsync(linkedToken).ConfigureAwait(false); if (chipInfo != null) { if (attempt > 1) { _logger.LogDebug( "LAN chip-info query succeeded on attempt {Attempt}/{Max}.", attempt, maxAttempts); } return chipInfo; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Defensively wrapping the task with WaitAsync is a good practice to ensure the total timeout is strictly enforced, even if the underlying ILanChipInfoProvider implementation fails to observe the cancellation token.	Low
More

---

Previous suggestions

Suggestions up to commit df91d5c

Category	Suggestion	Impact
Possible issue	Prevent timeout CTS constructor crashes Clamp LanChipInfoTotalTimeout to int.MaxValue milliseconds before creating the cancellation token source. This prevents potential exceptions if the timeout is set to an invalid value exceeding the platform's maximum limit. src/Daqifi.Core/Firmware/FirmwareUpdateService.cs [240-243] +var totalTimeout = _options.LanChipInfoTotalTimeout; + +// CancellationTokenSource(TimeSpan) throws if the timeout exceeds the platform maximum +// (approximately int.MaxValue milliseconds). Clamp defensively in case options validation +// was skipped or misconfigured. +var maxSupportedTimeout = TimeSpan.FromMilliseconds(int.MaxValue); +if (totalTimeout > maxSupportedTimeout) +{ + totalTimeout = maxSupportedTimeout; +} + using var timeoutCts = new CancellationTokenSource(totalTimeout); using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource( cancellationToken, timeoutCts.Token); var linkedToken = linkedCts.Token; Suggestion importance[1-10]: 2 __ Why: Defensively clamping LanChipInfoTotalTimeout against >24 days is overly defensive, as the native framework exception correctly surfaces improperly configured values if validation is bypassed.	Low

Suggestions up to commit b076e5f

Category	Suggestion	Impact
Possible issue	Prevent null version comparison crashes Add a defensive null check for latestWifi.Version before performing the version comparison. src/Daqifi.Core/Firmware/FirmwareUpdateService.cs [195-228] -if (latestWifi == null) +if (latestWifi == null || latestWifi.Version == null) { return new WifiFirmwareStatus { CurrentChipInfo = chipInfo, + LatestRelease = latestWifi, IsUpToDate = false, Reason = WifiFirmwareStatusReason.LatestReleaseUnavailable, }; } // Only the device-reported version needs parsing; latestWifi.Version // is already a strongly-typed FirmwareVersion from FirmwareDownloadService. // Re-parsing TagName would risk divergence from the canonical Version // (different tag prefix conventions, etc.) and cost an extra parse. if (!FirmwareVersion.TryParse(chipInfo.FwVersion, out var deviceVersion)) { return new WifiFirmwareStatus { CurrentChipInfo = chipInfo, LatestRelease = latestWifi, IsUpToDate = false, Reason = WifiFirmwareStatusReason.VersionUnparseable, }; } var isCurrent = deviceVersion >= latestWifi.Version; return new WifiFirmwareStatus { CurrentChipInfo = chipInfo, LatestRelease = latestWifi, IsUpToDate = isCurrent, Reason = isCurrent ? WifiFirmwareStatusReason.UpToDate : WifiFirmwareStatusReason.UpdateAvailable, }; Suggestion importance[1-10]: 3 __ Why: Adding a null check for latestWifi.Version is a valid but minor defensive measure, though it may be unnecessary depending on upstream type guarantees of the FirmwareReleaseInfo object.	Low

Suggestions up to commit 1ccb31b

Category	Suggestion	Impact
Possible issue	Prevent null version comparison crash Check if latestWifi.Version is null before comparing versions, and return a conservative fallback status if it is. src/Daqifi.Core/Firmware/FirmwareUpdateService.cs [152-174] // Only the device-reported version needs parsing; latestWifi.Version // is already a strongly-typed FirmwareVersion from FirmwareDownloadService. // Re-parsing TagName would risk divergence from the canonical Version // (different tag prefix conventions, etc.) and cost an extra parse. if (!FirmwareVersion.TryParse(chipInfo.FwVersion, out var deviceVersion)) { return new WifiFirmwareStatus { CurrentChipInfo = chipInfo, LatestRelease = latestWifi, IsUpToDate = false, Reason = WifiFirmwareStatusReason.VersionUnparseable, }; } +if (latestWifi.Version is null) +{ + return new WifiFirmwareStatus + { + CurrentChipInfo = chipInfo, + LatestRelease = latestWifi, + IsUpToDate = false, + Reason = WifiFirmwareStatusReason.LatestReleaseUnavailable, + }; +} + var isCurrent = deviceVersion >= latestWifi.Version; return new WifiFirmwareStatus { CurrentChipInfo = chipInfo, LatestRelease = latestWifi, IsUpToDate = isCurrent, Reason = isCurrent ? WifiFirmwareStatusReason.UpToDate : WifiFirmwareStatusReason.UpdateAvailable, }; Suggestion importance[1-10]: 3 __ Why: While adding a null check for latestWifi.Version provides extra safety, the PR author notes it is already a strongly-typed, populated object, making this a minor defensive improvement.	Low

✅ Suggestions up to commit f3a37ec

Category	Suggestion	Impact
Possible issue	✅ Serialize status probing with updates Suggestion Impact: CheckWifiFirmwareStatusAsync was changed to acquire the existing exclusive semaphore/lock (_operationLock) around the status probe, serializing it with firmware update I/O as suggested, though without adding the proposed RunExclusiveAsync overload. code diff: @@ -187,7 +187,22 @@ ArgumentNullException.ThrowIfNull(device); ThrowIfDisposed(); - return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false); + // Serialize device I/O with UpdateFirmwareAsync / UpdateWifiModuleAsync. + // GetLanChipInfoAsync runs a SCPI text exchange on the same transport + // those updates use; a concurrent caller would interleave on the wire + // and either corrupt the consumer swap or get partial replies. + // Acquired without the RunExclusiveAsync state check — a status probe + // is read-only and should be available to UI even when an update is + // in flight (it just waits for the in-flight I/O to release the lock). + await _operationLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false); + } + finally + { + _operationLock.Release(); + } } The suggestion recommends executing the status probe within the same exclusive lock as firmware updates to prevent race conditions during device I/O. It proposes adding a RunExclusiveAsync overload to serialize these operations safely without mutating the state. src/Daqifi.Core/Firmware/FirmwareUpdateService.cs [16-24] public async Task<WifiFirmwareStatus> CheckWifiFirmwareStatusAsync( IStreamingDevice device, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(device); ThrowIfDisposed(); - return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false); + return await RunExclusiveAsync( + ct => CheckWifiFirmwareStatusCoreAsync(device, ct), + cancellationToken).ConfigureAwait(false); } +private async Task<T> RunExclusiveAsync<T>( + Func<CancellationToken, Task<T>> operation, + CancellationToken cancellationToken) +{ + await _exclusiveSemaphore.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + return await operation(cancellationToken).ConfigureAwait(false); + } + finally + { + _exclusiveSemaphore.Release(); + } +} + Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential race condition in device communication and provides a standard synchronization solution.	Medium
	✅ Compare versions without tag parsing Suggestion Impact: The commit stopped parsing latestWifi.TagName, now only parses chipInfo.FwVersion into deviceVersion and compares it directly against latestWifi.Version; it also removed the redundant IsWifiVersionCurrent helper that re-parsed both strings. code diff: - if (!FirmwareVersion.TryParse(chipInfo.FwVersion, out _) || - !FirmwareVersion.TryParse(latestWifi.TagName, out _)) + // Only the device-reported version needs parsing; latestWifi.Version + // is already a strongly-typed FirmwareVersion from FirmwareDownloadService. + // Re-parsing TagName would risk divergence from the canonical Version + // (different tag prefix conventions, etc.) and cost an extra parse. + if (!FirmwareVersion.TryParse(chipInfo.FwVersion, out var deviceVersion)) { return new WifiFirmwareStatus { @@ -556,7 +574,7 @@ }; } - var isCurrent = IsWifiVersionCurrent(chipInfo.FwVersion, latestWifi.TagName); + var isCurrent = deviceVersion >= latestWifi.Version; return new WifiFirmwareStatus { CurrentChipInfo = chipInfo, @@ -564,17 +582,6 @@ IsUpToDate = isCurrent, Reason = isCurrent ? WifiFirmwareStatusReason.UpToDate : WifiFirmwareStatusReason.UpdateAvailable, }; - } - - private static bool IsWifiVersionCurrent(string deviceVersion, string latestTagName) - { - if (!FirmwareVersion.TryParse(deviceVersion, out var device) || - !FirmwareVersion.TryParse(latestTagName, out var latest)) - { - return false; - } - - return device >= latest; } The suggestion recommends using the already-parsed latestWifi.Version property instead of re-parsing latestWifi.TagName. This avoids redundant parsing, prevents potential string format issues (like 'v' prefixes), and simplifies the version comparison logic. src/Daqifi.Core/Firmware/FirmwareUpdateService.cs [137-156] -if (!FirmwareVersion.TryParse(chipInfo.FwVersion, out _) || - !FirmwareVersion.TryParse(latestWifi.TagName, out _)) +if (!FirmwareVersion.TryParse(chipInfo.FwVersion, out var deviceVersion)) { return new WifiFirmwareStatus { CurrentChipInfo = chipInfo, LatestRelease = latestWifi, IsUpToDate = false, Reason = WifiFirmwareStatusReason.VersionUnparseable, }; } -var isCurrent = IsWifiVersionCurrent(chipInfo.FwVersion, latestWifi.TagName); +var latestVersion = latestWifi.Version; +var isCurrent = deviceVersion >= latestVersion; + return new WifiFirmwareStatus { CurrentChipInfo = chipInfo, LatestRelease = latestWifi, IsUpToDate = isCurrent, Reason = isCurrent ? WifiFirmwareStatusReason.UpToDate : WifiFirmwareStatusReason.UpdateAvailable, }; Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out redundant and potentially unsafe string parsing for the version, since FirmwareReleaseInfo already has a strongly-typed Version property.	Medium

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit 1ccb31b

[qodo-code-review]

Persistent suggestions updated to latest commit 1ccb31b

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit bba154a

[qodo-code-review]

PR Code Suggestions ✨

Warning

/improve is deprecated. Use /agentic_review instead (removed after 2026-05-31).

No code suggestions found for the PR.

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit b076e5f

[qodo-code-review]

Persistent suggestions updated to latest commit b076e5f

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit df91d5c

[qodo-code-review]

Persistent suggestions updated to latest commit df91d5c

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit 2496323

[qodo-code-review]

Persistent suggestions updated to latest commit 2496323