fix(database): share one SQLite writer per owner and take write locks up front

.claude/plans/2026-09-09-bugsnag-6a4fde3.md

@@ -0,0 +1,53 @@
+# Bugsnag triage: Failed to persist conversation state [replace-feed]
+
+**id:** `6a4fde3…` · **URL:** https://app.bugsnag.com/1000710770-ontario-inc/flipcash-ios/errors/6a4fde33e96556123eb1f0ec
+**Triaged:** 2026-09-09 · **Status:** open · production
+**Last 7d:** 14 events · 3 users · last seen 2026-09-09
+**App version:** 2026.8.5 (local: 2026.9.1) · **Introduced in:** 1.13.0 (448)
+**Experts consulted:** /simplify (concurrency, testing and SwiftUI reviewed directly — those skills aren't installed here)
+
+## Root cause
+
+Four live `Database` instances wrote to one file: each `completeLogin` builds a `SessionContainer` (SessionAuthenticator.swift:389) that constructs a `Database` (SessionAuthenticator.swift:309, the only non-test site), each with its own writer `Connection` on the same path (Database.swift:34, :123). SQLite.swift serializes only *within* a `Connection` (Database.swift:16), so those are four real WAL writers.
+
+`replaceConversationFeed` opens a **deferred** transaction (Database+Conversations.swift:230; SQLite.swift's default, Connection.swift:366) that reads (:239) before writing (:242). A rival commit between snapshot and write fails the upgrade with `SQLITE_BUSY` immediately, bypassing the busy handler — hence a throw ~6 s after the containers spun up.
+
+The timeout would not have helped: `busyTimeout` is seconds (Connection.swift:417), so `2000` (Database.swift:36, commented "2 sec") arms ~33 minutes. Separate bug.
+
+Why five logins fire on one cold launch is **unverified**: SessionAuthenticator.swift:158–173 reads as linear and should stop on first success.
+
+## Evidence
+
+- nserror — `location=ConversationController.swift:persist(operation:_:):736`, exact against tag `flipcash-2026.8.5` (HEAD drifted to :747)
+- log `08:06:39–44 account-service` — "Logging in owner=DTAr…mLLW" ×5, five distinct `intentId`s
+- log `08:06:44–45 rates-controller` — "Rehydrated cached rates" ×4, `durationMs` 4.57/0.99/1.26/5.53: four real bootstraps, not repeated logging
+- `RatesController`/`WalletConnection` are built only in `createSessionContainer` (SessionAuthenticator.swift:235, :276) — it ran four times
+- log `08:06:50 ERROR` — "database is locked (code: 5) operation=replace-feed"; breadcrumbs show a cold launch, no user interaction
+- LogStore.swift:34–38 — release logs at `.info`, so `initializeState called` and `completeLogin` (both `debug`) never reach the report
+
+## Proposed direction
+
+Key one `Database` per owner in a store on `Container` (beside `accountManager`, Container.swift:16–18) and have `initializeDatabase` (SessionAuthenticator.swift:295) return the cached instance, so repeat logins share one writer. Add no explicit `close()`: SQLite.swift's `Connection` closes only in `deinit` (Connection.swift:144–145), and `HistoryController.sync()` holds the database in a `Task` with no `[weak self]` (HistoryController.swift:105–106), so a manual close would race in-flight work. Let ARC reclaim it. Separately, set `busyTimeout = 2` and mark read-then-write transactions `.immediate` so they take the write lock up front. The five logins are a separate defect; promote those two `debug` lines to `info` to capture the attempt count.
+
+## Verification
+
+`FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift`, built on `Database.makeTemp()` (FlipcashTests/TestSupport/Database+TestSupport.swift:14). Racing two writers is a false-green risk, so force it: hold `BEGIN IMMEDIATE` open on one connection, drop the other's `busyTimeout` to `0.05`, run the feed write, expect `SQLITE_BUSY`. Post-fix, assert identity rather than absence of an error — two `completeLogin` calls for one owner return the same `Database` (`===`).
+
+## Risk
+
+One shared `Database` means a stale container's in-flight writes land on the live store. That exposes the existing missing-teardown defect rather than adding one: `completeLogin` still never stops the previous container, only `logout()` does (SessionAuthenticator.swift:440–448).
+
+## Expert input
+
+- **/simplify**: the `completeLogin` guard sat at the wrong altitude — ownership, not a conditional, is the fix.
+- **concurrency**: `Connection` has no `close()`, only `deinit` — a keyed per-owner cache with ARC teardown avoids use-after-close.
+- **testing**: force contention explicitly rather than racing writers; assert `Database` identity per owner post-fix.
+- **SwiftUI**: `Database` is never `@Observable` or environment-injected, so moving it above `SessionContainer` is SwiftUI-invisible.
+
+## Next step
+
+If actioned: run `superpowers:writing-plans` against this file to expand into an implementation plan, and load `karpathy-guidelines` before writing code. The fix must land with that regression test, observed failing on unfixed code first — conventions and false-green traps in `references/regression-tests.md`.
+
+## Outcome
+
+The fix was written against the pre-refactor tree and rebased onto the App Group / `FlipcashStore` work (#753–#757), so the paths above are as of 2026-09-09 and several no longer resolve: the store lives in `FlipcashCore/Sources/FlipcashStore/`, and `initializeDatabase` became `DatabaseStore.database(for:)` reached through `Container`. `busyTimeout = 2` shipped with that stack rather than with this fix, so the separate bug the timeout section names is already closed.

Flipcash/Core/Container.swift

@@ -19,6 +19,7 @@ class Container {
     let betaFlags: BetaFlags
     let preferences: Preferences
     let notificationController: NotificationController
+    let databaseStore: DatabaseStore
 
     @ObservationIgnored lazy var sessionAuthenticator = SessionAuthenticator(container: self)
     @ObservationIgnored lazy var deepLinkController   = DeepLinkController(sessionAuthenticator: sessionAuthenticator)
@@ -38,6 +39,7 @@ class Container {
         self.betaFlags              = BetaFlags.shared
         self.preferences            = Preferences()
         self.notificationController = NotificationController()
+        self.databaseStore          = DatabaseStore()
 
         _ = sessionAuthenticator
     }

Flipcash/Core/Session/DatabaseStore.swift

@@ -0,0 +1,101 @@
+//
+//  DatabaseStore.swift
+//  Flipcash
+//
+
+import Foundation
+import FlipcashCore
+import FlipcashStore
+
+private let logger = Logger(label: "flipcash.database-store")
+
+/// Opens one ``Database`` per owner and hands back that same instance on every later request.
+///
+/// `SessionAuthenticator` builds a fresh `SessionContainer` on each login, and a logout that is
+/// followed by a login in the same process used to open a second `Database` on the same file. Both
+/// stayed alive — the old session's controllers hold their `Database` until they are released — so
+/// two connections competed for the write lock and the loser reported SQLITE_BUSY (Bugsnag
+/// 6a4fde3). Caching by owner means the second login reuses the first connection instead.
+///
+/// Entries are never evicted. One `Database` per account per process is bounded by how many
+/// accounts a person signs into before backgrounding the app, and dropping an entry would
+/// reintroduce exactly the second connection this exists to prevent.
+final class DatabaseStore {
+
+    private var databases: [PublicKey: Database] = [:]
+
+    /// The owner's `Database`, opened on first use: resolves the store location, migrates a legacy
+    /// store into the App Group container, and rebuilds the file when its schema is outdated.
+    func database(for owner: PublicKey) throws -> Database {
+        if let database = databases[owner] {
+            return database
+        }
+
+        // Resolved once. Two calls could in principle disagree — the container lookup is a
+        // system call, not a constant — and a migration that reads one directory while the
+        // store opens from another is the failure this avoids.
+        let location = StoreLocation.resolved()
+        let files = location.files(owner: owner)
+
+        if !location.isShared {
+            // The store still works; the notification extension cannot see it, so anything the
+            // extension prefetches is invisible to the app until this is fixed. That is an
+            // entitlement or provisioning problem, and it is silent without this.
+            ErrorReporting.captureError(
+                StoreError.appGroupUnavailable,
+                reason: "App Group container unavailable, database fell back to Application Support"
+            )
+        }
+
+        try createStoreDirectoryIfNeeded(at: location.directory)
+
+        switch StoreMigration.migrateIfNeeded(owner: owner, location: location) {
+        case .notNeeded, .alreadyMigrated:
+            break
+        case .migrated:
+            logger.info("Migrated the database into the App Group container.")
+        case .failed(let description):
+            // The migration cleaned up after itself, so what follows opens a fresh store and
+            // sync repopulates it. Worth reporting because the user pays for it in a full
+            // re-sync, and because it means the legacy store is still on disk.
+            ErrorReporting.captureError(
+                StoreError.migrationFailed(description),
+                reason: "Database migration to the App Group container failed"
+            )
+        }
+
+        // Currently we don't do migrations so every time
+        // the user version is outdated, we'll rebuild the
+        // database during sync.
+        let userVersion = (try? Database.userVersion(files: files)) ?? 0
+        let currentVersion = Database.schemaVersion
+        if currentVersion > userVersion {
+            try Database.deleteStore(files: files)
+            logger.error("Outdated user version, deleted database.")
+            try Database.setUserVersion(version: currentVersion, files: files)
+        }
+
+        let database = try Database(url: files.database)
+        databases[owner] = database
+        return database
+    }
+
+    /// Creates the store's directory when it is missing.
+    ///
+    /// `withIntermediateDirectories: true` where the old Application Support version passed
+    /// `false`: the App Group container root already exists once it resolves, so the call is
+    /// usually a no-op, and `true` also makes it succeed rather than throw in that case.
+    private func createStoreDirectoryIfNeeded(at directory: URL) throws {
+        if !FileManager.default.fileExists(atPath: directory.path) {
+            try FileManager.default.createDirectory(
+                at: directory,
+                withIntermediateDirectories: true
+            )
+        }
+    }
+
+    private enum StoreError: Error {
+        case appGroupUnavailable
+        case migrationFailed(String)
+    }
+}

Flipcash/Core/Session/SessionAuthenticator.swift

@@ -126,7 +126,7 @@ final class SessionAuthenticator {
     }
 
     private func initializeState(count: Int = 0, didAuthenticate: @escaping (UserAccount) -> Void, didFindRecentAccount: @escaping (KeyAccount) -> Void) {
-        logger.debug("initializeState called")
+        logger.info("initializeState called", metadata: ["count": "\(count)"])
 
         let userAccount = accountManager.fetchCurrentUserAccount()
         if let userAccount = userAccount {
@@ -224,7 +224,7 @@ final class SessionAuthenticator {
         let owner = initializedAccount.owner
         let ownerPublicKey = owner.authority.keyPair.publicKey
 
-        let database = try! initializeDatabase(owner: ownerPublicKey)
+        let database = try! container.databaseStore.database(for: ownerPublicKey)
 
         let historyController = HistoryController(
             container: container,
@@ -291,75 +291,6 @@ final class SessionAuthenticator {
         )
     }
 
-    // MARK: - Database -
-
-    private func initializeDatabase(owner: PublicKey) throws -> Database {
-        // Resolved once. Two calls could in principle disagree — the container lookup is a
-        // system call, not a constant — and a migration that reads one directory while the
-        // store opens from another is the failure this avoids.
-        let location = StoreLocation.resolved()
-        let files = location.files(owner: owner)
-
-        if !location.isShared {
-            // The store still works; the notification extension cannot see it, so anything the
-            // extension prefetches is invisible to the app until this is fixed. That is an
-            // entitlement or provisioning problem, and it is silent without this.
-            ErrorReporting.captureError(
-                StoreError.appGroupUnavailable,
-                reason: "App Group container unavailable, database fell back to Application Support"
-            )
-        }
-
-        try createStoreDirectoryIfNeeded(at: location.directory)
-
-        switch StoreMigration.migrateIfNeeded(owner: owner, location: location) {
-        case .notNeeded, .alreadyMigrated:
-            break
-        case .migrated:
-            logger.info("Migrated the database into the App Group container.")
-        case .failed(let description):
-            // The migration cleaned up after itself, so what follows opens a fresh store and
-            // sync repopulates it. Worth reporting because the user pays for it in a full
-            // re-sync, and because it means the legacy store is still on disk.
-            ErrorReporting.captureError(
-                StoreError.migrationFailed(description),
-                reason: "Database migration to the App Group container failed"
-            )
-        }
-
-        // Currently we don't do migrations so every time
-        // the user version is outdated, we'll rebuild the
-        // database during sync.
-        let userVersion = (try? Database.userVersion(files: files)) ?? 0
-        let currentVersion = Database.schemaVersion
-        if currentVersion > userVersion {
-            try Database.deleteStore(files: files)
-            logger.error("Outdated user version, deleted database.")
-            try Database.setUserVersion(version: currentVersion, files: files)
-        }
-
-        return try Database(url: files.database)
-    }
-
-    /// Creates the store's directory when it is missing.
-    ///
-    /// `withIntermediateDirectories: true` where the old Application Support version passed
-    /// `false`: the App Group container root already exists once it resolves, so the call is
-    /// usually a no-op, and `true` also makes it succeed rather than throw in that case.
-    private func createStoreDirectoryIfNeeded(at directory: URL) throws {
-        if !FileManager.default.fileExists(atPath: directory.path) {
-            try FileManager.default.createDirectory(
-                at: directory,
-                withIntermediateDirectories: true
-            )
-        }
-    }
-
-    private enum StoreError: Error {
-        case appGroupUnavailable
-        case migrationFailed(String)
-    }
-
     // MARK: - Login -
 
     func initialize(using mnemonic: MnemonicPhrase, isRegistration: Bool) async throws -> InitializedAccount {
@@ -426,7 +357,7 @@ final class SessionAuthenticator {
     }
 
     func completeLogin(with initializedAccount: InitializedAccount) {
-        logger.debug("completeLogin", metadata: ["owner": "\(initializedAccount.keyAccount.ownerPublicKey)"])
+        logger.info("completeLogin", metadata: ["owner": "\(initializedAccount.keyAccount.ownerPublicKey)"])
 
         let session = createSessionContainer(
             container: container,

FlipcashCore/Sources/FlipcashStore/Database+Conversations.swift

@@ -227,7 +227,10 @@ nonisolated extension Database {
         let c = ConversationTable()
         let m = ConversationMemberTable()
         let ids = conversations.map(\.id.data)
-        try writer.transaction {
+        // IMMEDIATE: this transaction reads before it writes. A DEFERRED one that reads first fails
+        // the write with SQLITE_BUSY at once when another writer holds the lock, without consulting
+        // the busy handler.
+        try writer.transaction(.immediate) {
             // Delete only the same-type conversations that dropped out of this feed, then upsert the
             // rest. `writeConversation` upserts the row (leaving `catchupCursor` untouched on conflict)
             // and replaces that conversation's members, so a surviving conversation keeps its event-log
@@ -281,7 +284,8 @@ nonisolated extension Database {
     /// until it does).
     public func persistMessages(_ messages: [ConversationMessage], cursor: UInt64, conversationID: ConversationID) throws {
         let c = ConversationTable()
-        try writer.transaction {
+        // IMMEDIATE: reads the current cursor before updating it (see replaceConversationFeed).
+        try writer.transaction(.immediate) {
             for message in messages {
                 try writeMessage(message, conversationId: conversationID.data)
             }

FlipcashCore/Sources/FlipcashStore/Database.swift

@@ -112,7 +112,8 @@ nonisolated open class Database: @unchecked Sendable {
         do {
             let connection = try writer
             let startChangeCount = connection.totalChanges
-            try connection.transaction { [unowned self] in
+            // IMMEDIATE: callers read and write inside the block; see replaceConversationFeed.
+            try connection.transaction(.immediate) { [unowned self] in
                 try block(self)
             }
             let endChangeCount = connection.totalChanges