From 85ef044af6ed45bc714224452f98548935ca35fc Mon Sep 17 00:00:00 2001
From: ecgang <eric@staqs.io>
Date: Sun, 26 Jul 2026 11:45:09 -0700
Subject: [PATCH] Courier spray: count offer spans instead of flagging one
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Cross-model review (agy) found a false negative in the overlap detector added
in the previous commit: with a boolean flag, a nested offer's `defer` clears it
while the OUTER offer is still between its scan and its commit, so every later
overlap with that outer offer goes unreported.

A depth count fixes it — report when the count is already above zero, then
increment; decrement on the way out.

Pinned by a test that provokes two overlaps against one outer offer. Reverting
the counter to flag semantics reports one of them, which is the mutation that
proves the test guards the behaviour rather than the implementation.

The same review confirmed no deadlock (the store queue is never held across
beginSprayOfferSpan, endSprayOfferSpan or the accepting callback), that bit 12
is wire-compatible with main's bits 8-10, and that notifyUI's non-async closure
does make `await` a compile error at the call site.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 bitchat/Services/Courier/CourierStore.swift | 16 ++++++-----
 bitchatTests/CourierStoreTests.swift        | 30 +++++++++++++++++++++
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/bitchat/Services/Courier/CourierStore.swift b/bitchat/Services/Courier/CourierStore.swift
index 9a1bb94a..91029e3b 100644
--- a/bitchat/Services/Courier/CourierStore.swift
+++ b/bitchat/Services/Courier/CourierStore.swift
@@ -152,7 +152,7 @@ final class CourierStore {
     private let queue = DispatchQueue(label: "chat.bitchat.courier.store")
 
     #if DEBUG
-    /// True while a spray offer is between its scan and its last commit.
+    /// How many spray offers are between their scan and their last commit.
     ///
     /// Both spray paths spend from one budget and both release the store queue
     /// across `accepting` (they must: it enters BLE/collections queues). The
@@ -161,7 +161,11 @@ final class CourierStore {
     /// isolation, because the violation that would actually ship is a
     /// suspension *inside* an already-MainActor block, which every isolation
     /// check passes.
-    private var sprayOfferInFlight = false
+    ///
+    /// A count rather than a flag: with a flag, a nested offer's `defer` clears
+    /// it while the outer offer is still in flight, so any later overlap with
+    /// that outer offer goes unreported.
+    private var sprayOffersInFlight = 0
     /// Test seam for the overlap detector, mirroring `_test_onOutboundPacket`
     /// in `BLEService`. Unset in normal debug runs, where an overlap trips
     /// `assertionFailure` instead.
@@ -173,10 +177,8 @@ final class CourierStore {
     private func beginSprayOfferSpan(_ function: StaticString = #function) {
         #if DEBUG
         queue.sync {
-            guard sprayOfferInFlight else {
-                sprayOfferInFlight = true
-                return
-            }
+            defer { sprayOffersInFlight += 1 }
+            guard sprayOffersInFlight > 0 else { return }
             if let hook = Self._test_onSprayOfferOverlap {
                 hook()
             } else {
@@ -193,7 +195,7 @@ final class CourierStore {
 
     private func endSprayOfferSpan() {
         #if DEBUG
-        queue.sync { sprayOfferInFlight = false }
+        queue.sync { sprayOffersInFlight = max(0, sprayOffersInFlight - 1) }
         #endif
     }
     private let fileURL: URL?
diff --git a/bitchatTests/CourierStoreTests.swift b/bitchatTests/CourierStoreTests.swift
index c2f9dc84..d36de0e7 100644
--- a/bitchatTests/CourierStoreTests.swift
+++ b/bitchatTests/CourierStoreTests.swift
@@ -792,6 +792,36 @@ struct CourierStoreTests {
         #expect(overlaps.count == 1)
     }
 
+    /// A nested offer must not end the OUTER offer's span when it returns.
+    ///
+    /// This is why the detector counts rather than sets a flag: with a flag,
+    /// the inner offer's `defer` clears it while the outer one is still between
+    /// its scan and its commit, so every later overlap with that outer offer
+    /// goes unreported. Two overlaps are provoked here; a flag reports one.
+    @Test func nestedOfferDoesNotEndTheOuterSpan() {
+        let store = makeStore()
+        let recipientKey = Data(repeating: 0xB0, count: 32)
+        let envelope = makeEnvelope(recipientKey: recipientKey).withCopies(8)
+        #expect(store.deposit(envelope, from: depositorA))
+        let courierA = Data(repeating: 0xC1, count: 32)
+        let courierB = Data(repeating: 0xC2, count: 32)
+        let courierC = Data(repeating: 0xC3, count: 32)
+
+        let overlaps = OverlapCounter()
+        CourierStore._test_onSprayOfferOverlap = { overlaps.count += 1 }
+        defer { CourierStore._test_onSprayOfferOverlap = nil }
+
+        _ = store.offerSprayCopies(to: courierA) { _ in
+            // B overlaps A and finishes; A is still in flight afterwards.
+            _ = store.offerSprayCopies(to: courierB) { _ in true }
+            // C therefore also overlaps A, and must be reported too.
+            _ = store.offerSprayCopies(to: courierC) { _ in true }
+            return true
+        }
+
+        #expect(overlaps.count == 2)
+    }
+
     /// Box so the detector's escaping closure can tally without capturing a
     /// local `var`.
     private final class OverlapCounter: @unchecked Sendable {