fix(sdcard): an SD download now owns the device, so a concurrent command can't corrupt the file by tylerkron · Pull Request #506 · daqifi/daqifi-core

tylerkron · 2026-08-12T17:35:55Z

What was wrong

If anything else touched the device while an SD card download was running, the downloaded file could come back silently corrupted. A UI status poll, an MCP get_device_status, an SD listing, or even a single fire-and-forget device.Send(...) from another thread was enough: the download had the transport stream to itself only by convention, not by any lock. A text query would open its own reader on the same stream and start eating the file's bytes; a bare Send() would put SCPI onto the wire mid-transfer and the device's reply would land inside the .bin. Nothing failed loudly — you got a truncated or garbled file, or the end-of-file marker was eaten and the download sat there until its 30-minute deadline expired. The other thread's own response was garbage too.

The download did hold a gate, but that gate only serialized downloads against each other.

How it was fixed

The raw-capture path the download runs on now takes the same device operation lock every other operation already takes. While a download is in flight, a text query from another thread waits for it, and a Send() from another thread is deferred and replayed afterwards — exactly what already happens around RunExclusiveAsync. Re-entry is nested, so the SD prepare/restore exchanges that already run under that lock don't deadlock against it. A capture also now counts as a consumer swap for the existing re-entrancy guard, so a second swap started from inside one is rejected instead of restarting the message consumer under a capture that still owns the stream.

Things you may want to push back on:

A download can now make an unrelated query wait up to 30 minutes. That is the deliberate trade: the alternative is the corruption above. Callers who cannot wait should pass a cancellation token — it is observed while queueing for the lock.
An abandoned transfer (the timeout path) holds the lock. It is cancelled on the way out, so the usual case unwinds and releases it. A read that never returns keeps the lock and the stream — at which point a blocked query was blocked on a stream nothing could have used anyway, and the reconnect that case already calls for clears it. Documented on DownloadSdCardFileAsync.
Validation moved to after the lock acquisition, so a session torn down while a capture queued is caught rather than handing out a dead stream. This adds TransportNotConnectedException (an InvalidOperationException) to the path, mirroring bug: SerialStreamTransport.Stream leaks raw InvalidOperationException when serial port closes mid-operation #238's fix for the text exchange.
An outbound drain was added at the start of the capture. The lock alone only parks sends that arrive after the capture opens; a command queued microseconds earlier would still have been written into it. This is the same barrier ExecuteAsync takes for investigate: per-device operation serialization for concurrent consumers #342, and costs at most 250 ms per download.
Merge order vs fix(device): cap the backlog of sends parked during a long exclusive operation #505. The deferred-send backlog is unbounded on main today (bug(device): Send() during a long exclusive operation parks unbounded deferred messages, then replays the entire backlog at once #492, fixed in fix(device): cap the backlog of sends parked during a long exclusive operation #505). This PR does not create that, but it does widen the window in which it can fill — so fix(device): cap the backlog of sends parked during a long exclusive operation #505 landing first is the tidier order.

Verification

12 new tests in DaqifiDeviceRawCaptureLockTests, covering all three of the issue's success criteria plus lock hygiene (nested re-entry, release on throw / on validation failure, cancellation while queued, teardown during the wait) and the two nested-swap rejections. Confirmed they catch the bugs rather than just passing: with the lock change reverted, 6 of 10 failed; with the swap guard reverted, 2 of 2 failed — and in both cases the tests that are guards correctly still passed.

Full suite green on net9.0 (2914 Core + 43 Mcp) and net10.0 (2914), 0 failures, 0 warnings.

Bench (non-destructive), fw 3.7.2 on /dev/cu.usbmodem1101, re-run after the review fix: connect → --show-status → 3 s @ 500 Hz → disconnect cycles returning 1177–1189 samples (this unit's known clock ratio), plus SD --sd-list (45 files) and --sd-storage — the SD prepare/finalize exchange path this change sits next to. sn and firmware unchanged, clean Disconnected. No SD:GET, no delete/format, no reboot, no firmware, serial only.

closes #493

Not merging — for review.

…493) The raw-capture path an SD download runs on suspended the protobuf consumer and took the transport stream while excluding nothing. A status poll from another thread acquired the text-exchange lock uncontended and started a second reader on that same stream; a plain Send() was not even deferred, so its bytes went out mid-transfer and the device's reply landed inside the downloaded file. ExecuteRawCaptureAsync now runs the same lock protocol ExecuteAsync does: nested re-entry via HoldsOperationLock, validation moved inside the lock so a session torn down during the wait is caught, an outbound drain so a command queued just before the capture cannot be written into it, and release on every exit path — which is also what replays the sends parked while the capture ran. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tylerkron · 2026-08-12T17:36:01Z

/agentic_review

qodo-code-review · 2026-08-12T17:37:23Z

PR Summary by Qodo

Fix SD download corruption by taking the device operation lock in raw capture

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Make raw-capture (SD download) acquire the device operation lock to prevent stream corruption.
• Defer and replay concurrent Send() traffic while a capture owns the transport stream.
• Add a dedicated test suite covering exclusion, deferral, cancellation, and lock release paths.

Diagram

graph TD
  A[Caller thread: SD download] --> B[Device operation lock] --> C[Raw capture engine] --> D[Transport stream]
  E[Caller thread: text query] --> B --> F[Text exchange]
  G[Caller thread: Send()] --> H[Outbound queue]
  C --> I[Drain outbound barrier]
  B --> H

High-Level Assessment

The following are alternative approaches to this PR:

1. Introduce a dedicated capture connection/transport

➕ Avoids blocking unrelated queries for long captures (e.g., 30-minute SD downloads)
➕ Hard isolation: no possibility of stream interleaving by design
➖ Requires device/firmware or protocol support for multiple connections
➖ Higher implementation complexity and operational surface (connection lifecycle, reconnection, resource limits)

2. Add a capture-mode gate that fails fast instead of blocking

➕ Callers get immediate feedback instead of waiting behind long downloads
➕ Can simplify UI responsiveness (polls can skip while capturing)
➖ Does not preserve current semantics where operations serialize and eventually run
➖ Requires every caller to handle a new error mode; risk of partial feature regressions

3. Multiplex stream access with a single reader and framed routing

➕ Could allow some concurrent logical operations while maintaining byte integrity
➕ Eliminates need for long lock holds in some cases
➖ Significant redesign: requires strict framing for all traffic and careful routing
➖ High risk in transport/protocol edge cases; larger testing burden

Recommendation: The PR’s approach (reuse the existing device operation lock + outbound drain barrier + nested re-entry) is the most pragmatic and safest fix for #493 because it aligns raw capture with the established ExecuteAsync concurrency contract. The long-wait tradeoff is explicit and is mitigated by honoring cancellation while queued; alternatives either require substantial architecture changes (multiplexing) or add disruptive new failure modes (fail-fast capture gate).

Files changed (4) +739 / -22

Bug fix (1) +115 / -18

TextExchangeEngine.csMake raw capture acquire operation lock, drain outbound queue, and validate post-wait +115/-18

Make raw capture acquire operation lock, drain outbound queue, and validate post-wait

• Changes ExecuteRawCaptureAsync to follow the same locking protocol as ExecuteAsync: acquire the operation lock (with nested re-entry), mark lock ownership so Send() defers, and release ownership to replay queued sends. Moves connection/transport validation inside the lock to close TOCTOU races during long waits, adds an outbound drain barrier to prevent pre-queued commands from interleaving into the capture, and introduces typed failure when the transport drops while queued.

src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs

Tests (1) +597 / -0

DaqifiDeviceRawCaptureLockTests.csAdd raw-capture lock/deferral regression test suite (#493) +597/-0

Add raw-capture lock/deferral regression test suite (#493)

• Introduces a comprehensive set of tests asserting that raw capture serializes with text exchanges and that concurrent Send() calls are deferred and replayed. Adds coverage for nested lock re-entry, cancellation while waiting, disconnect/teardown races, and lock release on all exit paths using an instrumented in-memory transport/stream.

src/Daqifi.Core.Tests/Device/DaqifiDeviceRawCaptureLockTests.cs

Documentation (2) +27 / -4

DaqifiDevice.csDocument raw-capture ownership semantics and lock behavior +12/-4

Document raw-capture ownership semantics and lock behavior

• Updates ExecuteRawCaptureAsync XML documentation to clarify that raw capture takes the device operation lock, defers concurrent Send() calls, and may block competing operations for long captures. Expands exception documentation to include shutdown/disconnect scenarios.

src/Daqifi.Core/Device/DaqifiDevice.cs

SdCardOperations.csDocument SD download locking tradeoffs and abandoned-transfer behavior +15/-0

Document SD download locking tradeoffs and abandoned-transfer behavior

• Extends SD download remarks to explicitly state that the transfer holds the device operation lock for the full duration and explains the resulting wait behavior for other operations. Documents how abandoned transfers can retain the lock/stream until reconnection in the pathological case of a read that never returns.

src/Daqifi.Core/Device/SdCard/SdCardOperations.cs

qodo-code-review · 2026-08-12T17:44:54Z

Code Review by Qodo

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

1. ~~Unguarded nested consumer swap~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

TextExchangeEngine.ExecuteRawCaptureAsync swaps the protobuf consumer but does not set the
AsyncLocal re-entrancy guard that prevents nested consumer swaps in ExecuteAsync. A nested raw
capture (or a nested text exchange invoked from rawAction) can therefore restart the protobuf
consumer while an outer capture still owns the stream, risking interleaved reads and corrupted
captured bytes.

Code

src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[R90-93]

+            // A flow that already owns the lock — an exclusive block, or the SD operations' own
+            // prepare/restore exchanges (#407) — runs nested rather than waiting on a semaphore it
+            // is itself holding, and leaves the release to the owner. Same rule as ExecuteAsync.
+            var ownsLock = !_host.HoldsOperationLock;

Relevance

●●● Strong
Team has accepted similar re-entrancy/concurrency guards to prevent nested consumer restarts and
stream corruption.
PR-#196
PR-#384

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
ExecuteRawCaptureAsync now explicitly supports nested operation-lock re-entry but performs a
consumer swap without setting the swap re-entrancy flag that ExecuteAsync relies on; nested calls
can therefore re-enter swapping logic and restart the consumer while an outer capture is still
active.
src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[79-186]
src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[211-257]

Agent prompt

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

## Issue description
`TextExchangeEngine.ExecuteRawCaptureAsync` performs a protobuf-consumer swap (stop consumer, hand out stream, restart consumer), but unlike `ExecuteAsync` it does not set a re-entrancy guard (`ITextExchangeHost.IsInsideTextExchange`). This allows nested swaps from within `rawAction` (or from callbacks it triggers), which can restart the protobuf consumer while an outer capture still owns the stream.

## Issue Context
- `ExecuteAsync` uses `_host.IsInsideTextExchange` to prevent nested consumer swaps.
- Raw capture runs nested when the caller already holds the operation lock, so nested calls on the same async flow won’t block.

## Fix Focus Areas
- src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[79-186]
- src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[211-257]

## Expected fix
- Introduce a shared guard for *any* consumer swap (text exchange and raw capture). Options:
 - Reuse `IsInsideTextExchange` but broaden its meaning (document it), setting it `true` for the entire raw-capture swap window, and throw a clear `InvalidOperationException` if already inside.
 - Preferably: add a new `ITextExchangeHost.IsInsideConsumerSwap` AsyncLocal and use it in both `ExecuteAsync` and `ExecuteRawCaptureAsync`.
- Add a regression test that attempts to start a nested raw capture from within a raw capture and asserts it throws (and does not restart the consumer mid-capture).

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

2. ~~Misleading re-entrancy message~~ ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New

Description

ExecuteRawCaptureAsync throws an InvalidOperationException claiming it cannot run “inside a text
exchange”, but the same IsInsideTextExchange flag is also set during raw captures, so nested raw
captures can surface an inaccurate diagnosis. This makes debugging nested capture failures harder
and conflicts with the new behavior/tests that reject raw-capture nesting.

Code

src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[R96-98]

+                throw new InvalidOperationException(
+                    "ExecuteRawCaptureAsync is not re-entrant on the same device and cannot run "
+                    + "inside a text exchange; both swap the device's message consumer.");

Relevance

●●● Strong
Team often accepts improving exception/diagnostic messages for accuracy and user guidance.
PR-#455
PR-#473

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The guard in ExecuteRawCaptureAsync throws a text-exchange-specific message when
_host.IsInsideTextExchange is true, but the same method sets _host.IsInsideTextExchange = true
for raw captures. Therefore, a nested raw capture will also hit this guard and incorrectly report
being inside a text exchange; the new tests explicitly exercise nested raw capture rejection.
src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[90-99]
src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[129-133]
src/Daqifi.Core.Tests/Device/DaqifiDeviceRawCaptureLockTests.cs[298-352]

Agent prompt

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

### Issue description
`TextExchangeEngine.ExecuteRawCaptureAsync` uses `_host.IsInsideTextExchange` as a generic “consumer swap in progress” guard (text exchange *or* raw capture). However, the thrown exception message says the call cannot run "inside a text exchange", which is misleading when a raw capture is nested inside another raw capture.

### Issue Context
- `_host.IsInsideTextExchange` is set to `true` for the duration of a raw capture.
- Nested raw captures are intentionally rejected.
- The diagnostic should reflect both possible contexts (raw capture or text exchange), or use a more general “consumer swap” phrasing.

### Fix Focus Areas
- src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[90-99]
- src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[129-133]
- src/Daqifi.Core.Tests/Device/DaqifiDeviceRawCaptureLockTests.cs[298-352]

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

3. Unbounded deferred send backlog 🐞 Bug ☼ Reliability

Description

By entering operation-lock ownership for the entire raw capture, Send() from other threads is
deferred for up to the SD download’s 30-minute budget and enqueued into an unbounded Queue<Action>.
Sustained Send() traffic during a long capture can therefore grow memory without bound and later
replay a large burst of stale commands after the capture completes.

Code

src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[R112-115]

+                // this frame's callees but never back to its caller, and the callees — the raw
+                // action and every Send() it makes — are exactly who must see it. See the remarks
+                // on ITextExchangeHost. This is also what starts deferring other flows' sends.
+                _host.EnterOperationLockOwnership();

Relevance

●● Moderate
They care about deferral safety, but bounding/dropping queued Send actions changes semantics;
unclear team preference.
PR-#421

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Raw capture now enters operation ownership (turning on Send deferral). Deferral enqueues actions
into an unbounded queue, and the API docs explicitly acknowledge captures can last 30 minutes,
increasing the window for backlog growth.
src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[90-116]
src/Daqifi.Core/Device/DaqifiDevice.cs[2134-2147]
src/Daqifi.Core/Device/DaqifiDevice.cs[1113-1129]
src/Daqifi.Core/Device/DaqifiDevice.cs[1933-1939]

Agent prompt

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

## Issue description
Raw capture now calls `EnterOperationLockOwnership()`, which turns on Send() deferral for the full capture duration. Deferred sends are stored in `_deferredSends` (a `Queue<Action>`) with no cap, so high-frequency callers can build an unbounded backlog during long captures (SD downloads can last ~30 minutes).

## Issue Context
Deferral is used to keep `Send()` non-blocking. With long-lived captures, this shifts the risk from wire corruption to unbounded memory growth and delayed/stale command replay.

## Fix Focus Areas
- src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[90-116]
- src/Daqifi.Core/Device/DaqifiDevice.cs[1113-1129]
- src/Daqifi.Core/Device/DaqifiDevice.cs[2134-2147]
- src/Daqifi.Core/Device/DaqifiDevice.cs[1933-1939]

## Expected fix
Implement an explicit backlog policy for deferred sends during long operations, such as:
- Hard cap on `_deferredSends` length (drop newest/oldest with a warning log), and/or
- Command coalescing for poll-like messages (replace previous of same kind), and/or
- Optionally allow callers to opt out (fail-fast) when a capture is active.
Add tests that verify:
- `Send()` remains non-blocking,
- backlog does not grow beyond the cap,
- overflow policy is applied deterministically.

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

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit 5dcd8e2

Results up to commit 4ba16d2 ⚖️ Balanced

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

1. ~~Unguarded nested consumer swap~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

TextExchangeEngine.ExecuteRawCaptureAsync swaps the protobuf consumer but does not set the
AsyncLocal re-entrancy guard that prevents nested consumer swaps in ExecuteAsync. A nested raw
capture (or a nested text exchange invoked from rawAction) can therefore restart the protobuf
consumer while an outer capture still owns the stream, risking interleaved reads and corrupted
captured bytes.

Code

src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[R90-93]

+            // A flow that already owns the lock — an exclusive block, or the SD operations' own
+            // prepare/restore exchanges (#407) — runs nested rather than waiting on a semaphore it
+            // is itself holding, and leaves the release to the owner. Same rule as ExecuteAsync.
+            var ownsLock = !_host.HoldsOperationLock;

Relevance

●●● Strong
Team has accepted similar re-entrancy/concurrency guards to prevent nested consumer restarts and
stream corruption.
PR-#196
PR-#384

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
ExecuteRawCaptureAsync now explicitly supports nested operation-lock re-entry but performs a
consumer swap without setting the swap re-entrancy flag that ExecuteAsync relies on; nested calls
can therefore re-enter swapping logic and restart the consumer while an outer capture is still
active.
src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[79-186]
src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[211-257]

Agent prompt

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

## Issue description
`TextExchangeEngine.ExecuteRawCaptureAsync` performs a protobuf-consumer swap (stop consumer, hand out stream, restart consumer), but unlike `ExecuteAsync` it does not set a re-entrancy guard (`ITextExchangeHost.IsInsideTextExchange`). This allows nested swaps from within `rawAction` (or from callbacks it triggers), which can restart the protobuf consumer while an outer capture still owns the stream.

## Issue Context
- `ExecuteAsync` uses `_host.IsInsideTextExchange` to prevent nested consumer swaps.
- Raw capture runs nested when the caller already holds the operation lock, so nested calls on the same async flow won’t block.

## Fix Focus Areas
- src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[79-186]
- src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[211-257]

## Expected fix
- Introduce a shared guard for *any* consumer swap (text exchange and raw capture). Options:
 - Reuse `IsInsideTextExchange` but broaden its meaning (document it), setting it `true` for the entire raw-capture swap window, and throw a clear `InvalidOperationException` if already inside.
 - Preferably: add a new `ITextExchangeHost.IsInsideConsumerSwap` AsyncLocal and use it in both `ExecuteAsync` and `ExecuteRawCaptureAsync`.
- Add a regression test that attempts to start a nested raw capture from within a raw capture and asserts it throws (and does not restart the consumer mid-capture).

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

2. Unbounded deferred send backlog 🐞 Bug ☼ Reliability

Description

By entering operation-lock ownership for the entire raw capture, Send() from other threads is
deferred for up to the SD download’s 30-minute budget and enqueued into an unbounded Queue<Action>.
Sustained Send() traffic during a long capture can therefore grow memory without bound and later
replay a large burst of stale commands after the capture completes.

Code

src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[R112-115]

+                // this frame's callees but never back to its caller, and the callees — the raw
+                // action and every Send() it makes — are exactly who must see it. See the remarks
+                // on ITextExchangeHost. This is also what starts deferring other flows' sends.
+                _host.EnterOperationLockOwnership();

Relevance

●● Moderate
They care about deferral safety, but bounding/dropping queued Send actions changes semantics;
unclear team preference.
PR-#421

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Raw capture now enters operation ownership (turning on Send deferral). Deferral enqueues actions
into an unbounded queue, and the API docs explicitly acknowledge captures can last 30 minutes,
increasing the window for backlog growth.
src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[90-116]
src/Daqifi.Core/Device/DaqifiDevice.cs[2134-2147]
src/Daqifi.Core/Device/DaqifiDevice.cs[1113-1129]
src/Daqifi.Core/Device/DaqifiDevice.cs[1933-1939]

Agent prompt

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

## Issue description
Raw capture now calls `EnterOperationLockOwnership()`, which turns on Send() deferral for the full capture duration. Deferred sends are stored in `_deferredSends` (a `Queue<Action>`) with no cap, so high-frequency callers can build an unbounded backlog during long captures (SD downloads can last ~30 minutes).

## Issue Context
Deferral is used to keep `Send()` non-blocking. With long-lived captures, this shifts the risk from wire corruption to unbounded memory growth and delayed/stale command replay.

## Fix Focus Areas
- src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs[90-116]
- src/Daqifi.Core/Device/DaqifiDevice.cs[1113-1129]
- src/Daqifi.Core/Device/DaqifiDevice.cs[2134-2147]
- src/Daqifi.Core/Device/DaqifiDevice.cs[1933-1939]

## Expected fix
Implement an explicit backlog policy for deferred sends during long operations, such as:
- Hard cap on `_deferredSends` length (drop newest/oldest with a warning log), and/or
- Command coalescing for poll-like messages (replace previous of same kind), and/or
- Optionally allow callers to opt out (fail-fast) when a capture is active.
Add tests that verify:
- `Send()` remains non-blocking,
- backlog does not grow beyond the cap,
- overflow policy is applied deterministically.

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

Round 1 review: ExecuteRawCaptureAsync stopped the protobuf consumer and took the stream without setting the AsyncLocal flag that marks "a swap is in progress on this flow". A nested capture — or a text exchange opened from inside a raw action — therefore ran, and its finally restarted the consumer while the outer capture still owned the stream. The flag now covers both swaps, and either one attempted inside the other fails with a clear InvalidOperationException instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tylerkron · 2026-08-12T17:53:07Z

/agentic_review

qodo-code-review · 2026-08-12T17:56:32Z

Code review by qodo was updated up to the latest commit 394a1da

Round 2 review: the message said the capture "cannot run inside a text exchange", but the flag it reads is also set by a raw capture — so a nested capture was diagnosed as something it was not. It now names the state (already inside a consumer swap) and both things that establish it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tylerkron · 2026-08-12T18:02:51Z

/agentic_review

qodo-code-review · 2026-08-12T18:04:54Z

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

tylerkron · 2026-08-12T18:15:33Z

Qodo-clean, CI green — ready for review.

Round 3 on head 5dcd8e2: no new findings, 0 unresolved review threads, both taken findings struck ✓ Resolved. Freshness proved by the bot's "review updated up to the latest commit 5dcd8e2" plus 20 SHA references to 5dcd8e2 against 1 each for the two older commits. Settle re-check at +11 min re-read both surfaces — byte-identical summary, still 0 unresolved threads. CI build pass, MERGEABLE / CLEAN.

Three rounds, three findings: the nested consumer swap (High) and the misleading re-entrancy message (Medium) were both valid and taken; the unbounded deferred-send backlog is real but is issue #492, already fixed in PR #505 — it stays un-struck in Qodo's summary because Qodo cannot tell "declined with reasoning" from "unaddressed", but its thread is resolved. See the merge-order note in the description.

tylerkron · 2026-08-12T18:17:05Z

Merge-order note is now moot: #505 merged at 17:59Z, while this PR was mid-review. The deferred-send cap (DefaultMaxDeferredSends = 1024, drop-oldest) is therefore already on main, so the widened deferral window this PR introduces lands on top of a bounded backlog rather than an unbounded one. Nothing to do here — this branch is based on baabf0c and merges cleanly; it just inherits the cap.

(#504 also merged, at 17:57Z.)

tylerkron requested a review from a team as a code owner August 12, 2026 17:35

qodo-code-review Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs

Comment thread src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs

qodo-code-review Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/Daqifi.Core/Device/Internal/TextExchangeEngine.cs Outdated

tylerkron added this pull request to the merge queue Aug 12, 2026

Merged via the queue into main with commit 27949d3 Aug 12, 2026
1 check passed

tylerkron deleted the fix/raw-capture-operation-lock-493 branch August 12, 2026 21:56

tylerkron mentioned this pull request Aug 12, 2026

perf(discovery): serial discovery no longer spends half its time waiting on its own reader thread #509

Merged

Navigation Menu

Uh oh!

fix(sdcard): an SD download now owns the device, so a concurrent command can't corrupt the file - #506

fix(sdcard): an SD download now owns the device, so a concurrent command can't corrupt the file#506
tylerkron merged 3 commits into
mainfrom
fix/raw-capture-operation-lock-493

tylerkron commented Aug 12, 2026 •

edited

Loading

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

qodo-code-review Bot commented Aug 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tylerkron commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was wrong

How it was fixed

Verification

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

tylerkron commented Aug 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tylerkron commented Aug 12, 2026 •

edited

Loading

qodo-code-review Bot commented Aug 12, 2026 •

edited

Loading