Device-test findings: keychain lock access, GeoDM dedup, sync log clarity (#1397)

Three findings from the July 7 locked-phone test sessions:

- Keychain items move from WhenUnlocked to AfterFirstUnlock: the mesh keeps
  running while the device is locked (identity-cache saves failed with
  -25308 throughout testing), and a wake-on-proximity relaunch via BLE
  state restoration must read the noise keys before the user unlocks. A
  one-time SecItemUpdate migration upgrades existing items (retried on next
  launch if the device is locked); backup semantics unchanged.

- GeoDM inbound messages dedup by message ID at the handler: outbox retries
  re-wrap the same message in fresh gift-wrap events, so relay-level
  event-ID dedup can't catch them and every copy ran full processing
  (3-6x per message observed). DELIVERED acks still go through
  markGeoDeliveryAckSent first, so re-sent copies from a lost ack are
  still answered.

- Periodic gossip sync logs now name the type group ("message+fragment").
  The five per-type schedules log identical lines when several fire in one
  maintenance tick, which reads as duplicated sends (misdiagnosed twice
  during testing).

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

bitchat/Services/KeychainManager.swift

@@ -15,7 +15,50 @@ final class KeychainManager: KeychainManagerProtocol {
     // Use consistent service name for all keychain items
     private let service = BitchatApp.bundleID
     private let appGroup = "group.\(BitchatApp.bundleID)"
-    
+
+    // AfterFirstUnlock, not WhenUnlocked: the mesh keeps running with the
+    // device locked (identity-cache saves failed with -25308 throughout
+    // locked-phone testing), and a wake-on-proximity relaunch via BLE state
+    // restoration must be able to read the noise keys before the user
+    // unlocks. Backup/sync semantics are unchanged (not ThisDeviceOnly).
+    private static let itemAccessibility = kSecAttrAccessibleAfterFirstUnlock
+
+    init() {
+        #if os(iOS)
+        migrateAccessibilityIfNeeded()
+        #endif
+    }
+
+    #if os(iOS)
+    /// One-time upgrade of items created under WhenUnlocked. New saves get
+    /// the right class on their own (saves are delete-then-add), but the
+    /// long-lived identity keys are written once and would otherwise stay
+    /// unreadable while the device is locked.
+    private func migrateAccessibilityIfNeeded() {
+        let flag = "keychain.accessibility.afterFirstUnlock.migrated"
+        guard !UserDefaults.standard.bool(forKey: flag) else { return }
+
+        let query: [String: Any] = [
+            kSecClass as String: kSecClassGenericPassword,
+            kSecAttrService as String: service
+        ]
+        let update: [String: Any] = [
+            kSecAttrAccessible as String: Self.itemAccessibility
+        ]
+        let status = SecItemUpdate(query as CFDictionary, update as CFDictionary)
+        switch status {
+        case errSecSuccess, errSecItemNotFound:
+            // Nothing to migrate on a fresh install; both are terminal.
+            UserDefaults.standard.set(true, forKey: flag)
+            SecureLogger.info("Keychain accessibility migrated to AfterFirstUnlock (status \(status))", category: .keychain)
+        default:
+            // Likely errSecInteractionNotAllowed (relaunched while locked) —
+            // leave the flag unset so the next launch retries.
+            SecureLogger.warning("Keychain accessibility migration deferred (status \(status))", category: .keychain)
+        }
+    }
+    #endif
+
     // MARK: - Identity Keys
     
     func saveIdentityKey(_ keyData: Data, forKey key: String) -> Bool {
@@ -62,7 +105,7 @@ final class KeychainManager: KeychainManagerProtocol {
             kSecAttrAccount as String: key,
             kSecValueData as String: data,
             kSecAttrService as String: service,
-            kSecAttrAccessible as String: kSecAttrAccessibleWhenUnlocked,
+            kSecAttrAccessible as String: Self.itemAccessibility,
             kSecAttrLabel as String: "bitchat-\(key)"
         ]
         #if os(macOS)
@@ -227,7 +270,7 @@ final class KeychainManager: KeychainManagerProtocol {
             kSecAttrAccount as String: key,
             kSecValueData as String: data,
             kSecAttrService as String: service,
-            kSecAttrAccessible as String: kSecAttrAccessibleWhenUnlocked,
+            kSecAttrAccessible as String: Self.itemAccessibility,
             kSecAttrLabel as String: "bitchat-\(key)"
         ]
         #if os(macOS)

bitchat/Sync/GossipSyncManager.swift

@@ -313,7 +313,7 @@ final class GossipSyncManager {
     private func sendPeriodicSync(for types: SyncTypeFlags) {
         // Unicast sync to connected peers to allow RSR attribution
         if let connectedPeers = delegate?.getConnectedPeers(), !connectedPeers.isEmpty {
-            SecureLogger.debug("Sending periodic sync to \(connectedPeers.count) connected peers", category: .sync)
+            SecureLogger.debug("Sending periodic sync (\(types.logDescription)) to \(connectedPeers.count) connected peers", category: .sync)
             for peerID in connectedPeers {
                 sendRequestSync(to: peerID, types: types)
             }

bitchat/Sync/SyncTypeFlags.swift

@@ -115,6 +115,15 @@ struct SyncTypeFlags: OptionSet {
         SyncTypeFlags(rawValue: rawValue & other.rawValue)
     }
 
+    /// Compact form for logs, e.g. "message+fragment". Without this, the
+    /// per-schedule periodic sync rounds log identical lines and read as
+    /// duplicated sends (misdiagnosed twice during July 2026 device testing).
+    var logDescription: String {
+        let types = toMessageTypes()
+        guard !types.isEmpty else { return "none" }
+        return types.map { String(describing: $0) }.joined(separator: "+")
+    }
+
     func toMessageTypes() -> [MessageType] {
         guard rawValue != 0 else { return [] }
         var types: [MessageType] = []

bitchat/ViewModels/ChatPrivateConversationCoordinator.swift

@@ -227,10 +227,29 @@ extension ChatViewModel: ChatPrivateConversationContext {
 final class ChatPrivateConversationCoordinator {
     private unowned let context: any ChatPrivateConversationContext
 
+    // Outbox retries re-wrap the same message in fresh gift-wrap events, so
+    // relay-level event-ID dedup can't catch them; track inbound GeoDM
+    // message IDs so each copy past the first costs one (already-deduped)
+    // ack check and nothing else.
+    private var seenInboundGeoDMIDs: Set<String> = []
+    private var seenInboundGeoDMOrder: [String] = []
+    private static let seenInboundGeoDMCap = 512
+
     init(context: any ChatPrivateConversationContext) {
         self.context = context
     }
 
+    /// Returns `false` if this GeoDM message ID was already handled.
+    private func markInboundGeoDMSeen(_ messageId: String) -> Bool {
+        guard !seenInboundGeoDMIDs.contains(messageId) else { return false }
+        seenInboundGeoDMIDs.insert(messageId)
+        seenInboundGeoDMOrder.append(messageId)
+        if seenInboundGeoDMOrder.count > Self.seenInboundGeoDMCap {
+            seenInboundGeoDMIDs.remove(seenInboundGeoDMOrder.removeFirst())
+        }
+        return true
+    }
+
     func sendPrivateMessage(_ content: String, to peerID: PeerID) {
         guard !content.isEmpty else { return }
 
@@ -408,10 +427,15 @@ final class ChatPrivateConversationCoordinator {
         guard let pm = PrivateMessagePacket.decode(from: payload.data) else { return }
         let messageId = pm.messageID
 
-        SecureLogger.info("GeoDM: recv PM <- sender=\(senderPubkey.prefix(8))… mid=\(messageId.prefix(8))…", category: .session)
-
+        // Ack before the dedup guard: a re-sent copy means the sender may not
+        // have our DELIVERED yet, and markGeoDeliveryAckSent dedups the
+        // actual sends.
         sendDeliveryAckIfNeeded(to: messageId, senderPubKey: senderPubkey, from: id)
 
+        guard markInboundGeoDMSeen(messageId) else { return }
+
+        SecureLogger.info("GeoDM: recv PM <- sender=\(senderPubkey.prefix(8))… mid=\(messageId.prefix(8))…", category: .session)
+
         if context.isNostrBlocked(pubkeyHexLowercased: senderPubkey) {
             return
         }