From 54c85876b39c0ceab3ce351efdbf2921a9e9a8a6 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 17:29:53 -0400 Subject: [PATCH 1/8] docs(plans): triage brief and plan for Bugsnag 6a4fde3 database lock --- .../plans/2026-09-09-bugsnag-6a4fde3-plan.md | 689 ++++++++++++++++++ .claude/plans/2026-09-09-bugsnag-6a4fde3.md | 49 ++ 2 files changed, 738 insertions(+) create mode 100644 .claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md create mode 100644 .claude/plans/2026-09-09-bugsnag-6a4fde3.md diff --git a/.claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md b/.claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md new file mode 100644 index 000000000..3aa70833f --- /dev/null +++ b/.claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md @@ -0,0 +1,689 @@ +# One Database Per Owner (Bugsnag 6a4fde3) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stop `replaceConversationFeed` failing with `database is locked (code: 5)` when repeat logins in one process open several SQLite writers on the same owner store. + +**Architecture:** A new `DatabaseStore` on `Container` opens one `Database` per owner and returns the cached instance on every later login, so all `SessionContainer`s for an owner share one writer `Connection`. As defence in depth, read-then-write transactions become `BEGIN IMMEDIATE` so the busy handler is consulted instead of an instant `SQLITE_BUSY`, and `busyTimeout` is corrected from ~33 minutes to the 2 seconds its comment promises. Two `debug` login logs are promoted to `info` so the still-unexplained five-login trigger shows up in future Bugsnag reports. + +**Tech Stack:** Swift 6 (app target has `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`), SQLite.swift (`Connection`, `TransactionMode`), Swift Testing (`@Suite`, `@Test`, `.bug`), `./Scripts/test.sh` on the iPhone 17 simulator. + +**Spec:** `.claude/plans/2026-09-09-bugsnag-6a4fde3.md` (the triage brief). Read it first. + +**Not available in this environment:** the brief says to load `karpathy-guidelines` before writing code. That skill is not installed here (`Skill` returns "Unknown skill"). Follow the project's `.claude/docs/hard-rules.md` and `.claude/reflections/index.md` instead; the relevant reflection is 2026-07-11 (regression test at the wrong layer). + +--- + +## File map + +| File | Change | Responsibility | +|---|---|---| +| `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` | Create | Regression suite: contention at the crash layer, busy-timeout units, one `Database` per owner | +| `Flipcash/Core/Controllers/Database/DatabaseStore.swift` | Create | Per-owner `Database` cache; owns the open/version-check logic that today lives in `SessionAuthenticator.initializeDatabase` | +| `Flipcash/Core/Container.swift` | Modify | Hold `let databaseStore` beside the other process-lifetime units | +| `Flipcash/Core/Session/SessionAuthenticator.swift` | Modify | `createSessionContainer` asks the store; delete `initializeDatabase` / `createApplicationSupportIfNeeded`; promote two logs to `info` | +| `Flipcash/Core/Controllers/Database/Database.swift` | Modify | `busyTimeout = 2`; `Database.transaction` helper uses `.immediate` | +| `Flipcash/Core/Controllers/Database/Database+Conversations.swift` | Modify | `.immediate` on the two read-then-write transactions | + +Nothing else changes. No schema change, so `SQLiteVersion` stays as is. No new SPM dependency, so `Package.resolved` is untouched. + +Both `Flipcash/` and `FlipcashTests/` are `PBXFileSystemSynchronizedRootGroup`s (`Code.xcodeproj/project.pbxproj:136-137`), so new files under them join their targets without editing the project file. + +## How the tests are run + +`./Scripts/test.sh /[/]` builds and runs on the iPhone 17 simulator. The suite identifier is the Swift type name, not the display string: + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3 +``` + +A single test: + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3/replaceFeed_rivalWriterHoldsLock_waitsThenCommits +``` + +`xcodebuild` ends with `** TEST SUCCEEDED **` or `** TEST FAILED **`. Never run the full `AllTargets` plan; that is the user's job. + +--- + +### Task 0: Branch + +**Files:** none + +- [ ] **Step 1: Create the fix branch from the current HEAD** + +The worktree sits on `claude/flipcash-ios-error-triage-7e50ad` at `8042ff9d`, which is `main`. The user's rules forbid `claude/`-prefixed branch names for the work itself. + +```bash +git checkout -b fix/database-per-owner +``` + +Expected: `Switched to a new branch 'fix/database-per-owner'` + +- [ ] **Step 2: Confirm the tree is clean apart from the two plan files** + +```bash +git status --short +``` + +Expected: + +``` +?? .claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md +?? .claude/plans/2026-09-09-bugsnag-6a4fde3.md +``` + +- [ ] **Step 3: Commit the triage brief and this plan** + +```bash +git add .claude/plans/2026-09-09-bugsnag-6a4fde3.md .claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md +git commit -m "docs(plans): triage brief and plan for Bugsnag 6a4fde3 database lock" +``` + +--- + +### Task 1: Regression test at the crash layer (red) + +This test reproduces the production failure through `Database.replaceConversationFeed`, the exact call that threw in `ConversationController.persist(operation: "replace-feed")`. A second `Database` on the same file plays the role of the other `SessionContainer`s' writers. + +Why it discriminates: on unfixed code the transaction is `BEGIN DEFERRED`. The `SELECT` at `Database+Conversations.swift:239` opens a read snapshot; the `DELETE` at `:242` then needs the write lock the rival holds, and SQLite returns `SQLITE_BUSY` **without invoking the busy handler** because the connection already has a read transaction open. So the call throws at t≈0 even though the rival releases the lock 200 ms later. After the fix the transaction is `BEGIN IMMEDIATE`, which takes the write lock first, so the busy handler waits, the rival commits at 200 ms, and the feed write goes through. + +**Files:** +- Create: `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` + +- [ ] **Step 1: Write the failing test** + +```swift +// +// Regression_6a4fde33e96556123eb1f0ec.swift +// FlipcashTests +// +// "Failed to persist conversation state [replace-feed]" — database is locked (code: 5). +// One cold launch ran completeLogin five times for the same owner; each built a +// SessionContainer with its own Database, so four writer Connections shared one +// SQLite file. replaceConversationFeed ran a DEFERRED transaction that read before +// it wrote: once a rival writer held the lock, the snapshot upgrade returned +// SQLITE_BUSY immediately, bypassing the busy handler. +// +// Fix: Container.databaseStore hands out one Database per owner, so repeat logins +// share a single writer; read-then-write transactions take the write lock up front +// with BEGIN IMMEDIATE; busyTimeout is 2 seconds rather than 2000. +// + +import Foundation +import Testing +import FlipcashCore +@testable import Flipcash + +@MainActor +@Suite("Regression: 6a4fde3 – replace-feed fails SQLITE_BUSY against a rival writer", .bug("6a4fde33e96556123eb1f0ec")) +struct Regression_6a4fde3 { + + private func conversation(_ byte: UInt8) -> Conversation { + Conversation(id: .test(byte), members: [], lastMessage: nil, lastActivity: Date(timeIntervalSince1970: 100)) + } + + @Test("replace-feed waits for a rival writer to commit instead of failing the snapshot upgrade") + func replaceFeed_rivalWriterHoldsLock_waitsThenCommits() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + // Seed a row so the second feed has something to delete — the read-then-write path. + try database.replaceConversationFeed([conversation(1)], type: .contactDm) + + // A second Database on the same file is exactly what each extra SessionContainer opened. + let rival = try Database(url: url) + try rival.writer.run("BEGIN IMMEDIATE TRANSACTION") + let release = Task.detached { + try await Task.delay(milliseconds: 200) + try rival.writer.run("COMMIT TRANSACTION") + } + + try database.replaceConversationFeed([conversation(2)], type: .contactDm) + try await release.value + + let ids = try database.getConversations().map(\.id) + #expect(ids == [.test(2)]) + } +} +``` + +- [ ] **Step 2: Run it and watch it fail at the crash layer** + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3/replaceFeed_rivalWriterHoldsLock_waitsThenCommits +``` + +Expected: `** TEST FAILED **`. The failure for `replaceFeed_rivalWriterHoldsLock_waitsThenCommits` must be a thrown error whose text contains `database is locked` and `(code: 5)`, the production signature. SQLite.swift includes the failing statement in the description when it has one, so the local text will read `database is locked (DELETE FROM "conversations" ...) (code: 5)` where production showed only `database is locked (code: 5)`; the shared part is what matters. If the test instead fails on the `#expect`, or passes, stop: the reproduction is not hitting the deferred-snapshot path and the test needs rethinking, not the fix. + +- [ ] **Step 3: Commit the red test** + +```bash +git add FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift +git commit -m "test(database): reproduce replace-feed SQLITE_BUSY against a rival writer (6a4fde3)" +``` + +--- + +### Task 2: Immediate transactions for read-then-write paths (green) + +**Files:** +- Modify: `Flipcash/Core/Controllers/Database/Database+Conversations.swift:230` and `:284` +- Modify: `Flipcash/Core/Controllers/Database/Database.swift:56` +- Test: `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` + +Three sites read inside the transaction before writing: `replaceConversationFeed` (the `SELECT` of doomed ids), `persistMessages` (the `pluck` of the current cursor), and every caller of the `Database.transaction` helper (`Database+Balance.swift:158`, `:203`, `Database+Rates.swift:45`, `Database+VerifiedProtos.swift:21`, `:56` all read rows before upserting). The remaining `writer.transaction {` sites begin with a `DELETE` or `INSERT`, which takes the write lock as its first statement and so already consults the busy handler; leave them alone. + +- [ ] **Step 1: Make `replaceConversationFeed` immediate** + +In `Database+Conversations.swift`, change line 230 from: + +```swift + try writer.transaction { +``` + +to: + +```swift + // 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) { +``` + +- [ ] **Step 2: Make `persistMessages` immediate** + +In `Database+Conversations.swift`, change line 284 (inside `persistMessages`) from: + +```swift + try writer.transaction { +``` + +to: + +```swift + try writer.transaction(.immediate) { +``` + +- [ ] **Step 3: Make the `Database.transaction` helper immediate** + +In `Database.swift`, change line 56 from: + +```swift + try writer.transaction { [unowned self] in +``` + +to: + +```swift + try writer.transaction(.immediate) { [unowned self] in +``` + +- [ ] **Step 4: Run the regression test and see it pass** + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3/replaceFeed_rivalWriterHoldsLock_waitsThenCommits +``` + +Expected: `** TEST SUCCEEDED **`. The test now takes a little over 200 ms because `BEGIN IMMEDIATE` waits for the rival's commit. + +- [ ] **Step 5: Run the existing database and conversation suites to catch a regression in the helper change** + +```bash +./Scripts/test.sh FlipcashTests/DatabaseBalanceUpsertTests FlipcashTests/DatabaseLiveSupplyTests FlipcashTests/ConversationControllerTests +``` + +Those are the struct names at `FlipcashTests/Database/Database+BalanceUpsertTests.swift:12`, `FlipcashTests/Database/Database+LiveSupplyTests.swift:15`, and `FlipcashTests/ConversationControllerTests.swift:13`; the first two go through the `Database.transaction` helper. + +Expected: `** TEST SUCCEEDED **`. + +- [ ] **Step 6: Commit** + +```bash +git add Flipcash/Core/Controllers/Database/Database+Conversations.swift Flipcash/Core/Controllers/Database/Database.swift +git commit -m "fix(database): take the write lock up front in read-then-write transactions" +``` + +--- + +### Task 3: Busy timeout in seconds (red, then green) + +`Connection.busyTimeout` is a `Double` in **seconds** (SQLite.swift `Connection.swift:415-419` multiplies by 1 000 before calling `sqlite3_busy_timeout`). `Database.init` sets `2000` with the comment `// 2 sec`, which arms a ~33 minute wait. + +**Files:** +- Modify: `Flipcash/Core/Controllers/Database/Database.swift:36` and `:42` +- Test: `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` + +- [ ] **Step 1: Add the failing test** + +Append inside `struct Regression_6a4fde3`, after `replaceFeed_rivalWriterHoldsLock_waitsThenCommits`: + +```swift + @Test("busy timeout is two seconds, not two thousand") + func busyTimeout_isTwoSeconds() throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + + // SQLite.swift's busyTimeout is in seconds; 2000 arms a ~33 minute wait. + #expect(database.writer.busyTimeout == 2) + #expect(database.reader.busyTimeout == 2) + } +``` + +- [ ] **Step 2: Run it and see it fail** + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3/busyTimeout_isTwoSeconds +``` + +Expected: `** TEST FAILED **` with `Expectation failed: (database.writer.busyTimeout → 2000.0) == 2`. + +- [ ] **Step 3: Fix the two assignments** + +In `Database.swift`, replace lines 36 and 42: + +```swift + writer.busyTimeout = 2000 // 2 sec +``` + +becomes + +```swift + writer.busyTimeout = 2 // seconds +``` + +and + +```swift + reader.busyTimeout = 2000 // 2 Sec +``` + +becomes + +```swift + reader.busyTimeout = 2 // seconds +``` + +- [ ] **Step 4: Run the whole regression suite and see it pass** + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3 +``` + +Expected: `** TEST SUCCEEDED **`, two tests passing. + +- [ ] **Step 5: Commit** + +```bash +git add Flipcash/Core/Controllers/Database/Database.swift FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift +git commit -m "fix(database): busy timeout is in seconds, arm 2s not 33min" +``` + +--- + +### Task 4: `DatabaseStore`, one `Database` per owner (red, then green) + +This is the root-cause fix. The identity test is written against `DatabaseStore` rather than `SessionAuthenticator.completeLogin` because `completeLogin` builds a full `SessionContainer` whose `HistoryController.sync()` and `PushController` hit the live network from a unit test. Task 5 is the wiring that makes `completeLogin` go through the store, and its verification step greps that the store is the only remaining `Database` constructor call in the app target. + +**Files:** +- Create: `Flipcash/Core/Controllers/Database/DatabaseStore.swift` +- Test: `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` + +- [ ] **Step 1: Add the failing tests** + +Append inside `struct Regression_6a4fde3`, after `busyTimeout_isTwoSeconds`: + +```swift + /// `DatabaseStore` writes to the real Application Support directory, so each test + /// uses a throwaway owner and removes that owner's store and version file after. + private func withThrowawayOwner(_ body: (PublicKey) throws -> Void) throws { + let owner = KeyPair.generate()!.publicKey + defer { + try? Database.deleteStore(owner: owner) + try? FileManager.default.removeItem(at: .versionFile(owner: owner)) + } + try body(owner) + } + + @Test("one owner gets the same Database on every login") + func databaseStore_sameOwnerTwice_returnsOneInstance() throws { + try withThrowawayOwner { owner in + let store = DatabaseStore() + + let first = try store.database(for: owner) + let second = try store.database(for: owner) + + #expect(first === second) + } + } + + @Test("different owners get different Databases") + func databaseStore_twoOwners_returnsDistinctInstances() throws { + try withThrowawayOwner { alice in + try withThrowawayOwner { bob in + let store = DatabaseStore() + + let aliceDatabase = try store.database(for: alice) + let bobDatabase = try store.database(for: bob) + + #expect(aliceDatabase !== bobDatabase) + } + } + } +``` + +- [ ] **Step 2: Run the suite and see it fail to compile** + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3 +``` + +Expected: `** TEST FAILED **` (build failure) with `cannot find 'DatabaseStore' in scope`. This is the expected red state: the type does not exist yet. + +- [ ] **Step 3: Create `DatabaseStore`** + +Create `Flipcash/Core/Controllers/Database/DatabaseStore.swift`. The body of `database(for:)` is `SessionAuthenticator.initializeDatabase` (`SessionAuthenticator.swift:295-310`) and `createApplicationSupportIfNeeded` (`:312-319`) moved verbatim, with the cache lookup in front. + +```swift +// +// DatabaseStore.swift +// Flipcash +// + +import Foundation +import FlipcashCore + +private let logger = Logger(label: "flipcash.database-store") + +/// Opens one `Database` per owner and returns that same instance on every later request, +/// so repeat logins in one process share a single SQLite writer instead of contending for the file. +final class DatabaseStore { + + private var databases: [PublicKey: Database] = [:] + + /// The owner's `Database`, opened (and rebuilt if its on-disk version is outdated) on first use. + func database(for owner: PublicKey) throws -> Database { + if let database = databases[owner] { + return database + } + + try createApplicationSupportIfNeeded() + + // 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(owner: owner)) ?? 0 + let currentVersion = try InfoPlist.value(for: "SQLiteVersion").integer() + if currentVersion > userVersion { + try Database.deleteStore(owner: owner) + logger.error("Outdated user version, deleted database.") + try Database.setUserVersion(version: currentVersion, owner: owner) + } + + let database = try Database(url: .dataStore(owner: owner)) + databases[owner] = database + return database + } + + private func createApplicationSupportIfNeeded() throws { + if !FileManager.default.fileExists(atPath: URL.applicationSupportDirectory.path) { + try FileManager.default.createDirectory( + at: .applicationSupportDirectory, + withIntermediateDirectories: false + ) + } + } +} +``` + +Notes for the implementer: +- `DatabaseStore` is `@MainActor` by the app target's default isolation, the same as `Container` and `SessionAuthenticator.createSessionContainer`, so the dictionary needs no lock. +- `PublicKey` is already used as a dictionary key elsewhere (`TokenCardStack.swift:83`), so it is `Hashable`. +- There is deliberately no `close()` or eviction. SQLite.swift's `Connection` closes only in `deinit`, and `HistoryController.sync()` (`HistoryController.swift:105`) holds the database in a `Task` with no `[weak self]`, so an explicit close would race in-flight work. A cached `Database` lives for the process; the cost is two open connections per owner ever logged in. + +- [ ] **Step 4: Run the suite and see it pass** + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3 +``` + +Expected: `** TEST SUCCEEDED **`, four tests passing. + +- [ ] **Step 5: Commit** + +```bash +git add Flipcash/Core/Controllers/Database/DatabaseStore.swift FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift +git commit -m "feat(database): DatabaseStore caches one Database per owner" +``` + +--- + +### Task 5: Wire the store through `Container` and `SessionAuthenticator` + +**Files:** +- Modify: `Flipcash/Core/Container.swift:16-21`, `:35-40` +- Modify: `Flipcash/Core/Session/SessionAuthenticator.swift:226`, `:293-319` + +- [ ] **Step 1: Hold the store on `Container`** + +In `Container.swift`, add a property after `let notificationController: NotificationController` (line 21): + +```swift + let notificationController: NotificationController + let databaseStore: DatabaseStore +``` + +and initialise it in `init()` after `self.notificationController = NotificationController()` (line 40): + +```swift + self.notificationController = NotificationController() + self.databaseStore = DatabaseStore() +``` + +- [ ] **Step 2: Ask the store in `createSessionContainer`** + +In `SessionAuthenticator.swift`, change line 226 from: + +```swift + let database = try! initializeDatabase(owner: ownerPublicKey) +``` + +to: + +```swift + let database = try! container.databaseStore.database(for: ownerPublicKey) +``` + +- [ ] **Step 3: Delete the moved code** + +In `SessionAuthenticator.swift`, delete lines 293 to 320 in full, that is the `// MARK: - Database -` header, `initializeDatabase(owner:)`, and `createApplicationSupportIfNeeded()`: + +```swift + // MARK: - Database - + + private func initializeDatabase(owner: PublicKey) throws -> Database { + try createApplicationSupportIfNeeded() + + // 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(owner: owner)) ?? 0 + let currentVersion = try InfoPlist.value(for: "SQLiteVersion").integer() + if currentVersion > userVersion { + try Database.deleteStore(owner: owner) + logger.error("Outdated user version, deleted database.") + try Database.setUserVersion(version: currentVersion, owner: owner) + } + + return try Database(url: .dataStore(owner: owner)) + } + + private func createApplicationSupportIfNeeded() throws { + if !FileManager.default.fileExists(atPath: URL.applicationSupportDirectory.path) { + try FileManager.default.createDirectory( + at: .applicationSupportDirectory, + withIntermediateDirectories: false + ) + } + } + +``` + +Leave the `// MARK: - Login -` header that follows in place. + +- [ ] **Step 4: Verify the store is now the only `Database` constructor in the app target** + +```bash +grep -rn "Database(url" Flipcash --include=*.swift +``` + +Expected, exactly one line: + +``` +Flipcash/Core/Controllers/Database/DatabaseStore.swift:37: let database = try Database(url: .dataStore(owner: owner)) +``` + +(The line number may differ by one or two; the file must be the only match.) + +- [ ] **Step 5: Build the app** + +```bash +./Scripts/build.sh +``` + +Expected: `** BUILD SUCCEEDED **`. + +- [ ] **Step 6: Run the regression suite plus the suites that construct a `Container`** + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3 FlipcashTests/DeepLinkControllerTests +``` + +`DeepLinkControllerTests.swift:16` builds `SessionAuthenticator(container: Container())`, so it exercises the new `Container.init` path. + +Expected: `** TEST SUCCEEDED **`. + +- [ ] **Step 7: Commit** + +```bash +git add Flipcash/Core/Container.swift Flipcash/Core/Session/SessionAuthenticator.swift +git commit -m "fix(session): share one Database per owner across repeat logins (6a4fde3)" +``` + +--- + +### Task 6: Promote the two login logs to `info` + +Release builds bootstrap logging at `.info` (`FlipcashCore/Sources/FlipcashCore/Logging/LogStore.swift:34-38`), so the `debug` lines that would have shown *why* one launch ran `completeLogin` five times never reached the Bugsnag report. This task has no test: it changes a log level and adds metadata, with no behaviour to assert. + +**Files:** +- Modify: `Flipcash/Core/Session/SessionAuthenticator.swift:128`, `:387` + +- [ ] **Step 1: Log each `initializeState` attempt at `info` with its retry count** + +Change line 128 from: + +```swift + logger.debug("initializeState called") +``` + +to: + +```swift + logger.info("initializeState called", metadata: ["count": "\(count)"]) +``` + +- [ ] **Step 2: Log `completeLogin` at `info`** + +Change line 387 from: + +```swift + logger.debug("completeLogin", metadata: ["owner": "\(initializedAccount.keyAccount.ownerPublicKey)"]) +``` + +to: + +```swift + logger.info("completeLogin", metadata: ["owner": "\(initializedAccount.keyAccount.ownerPublicKey)"]) +``` + +- [ ] **Step 3: Build** + +```bash +./Scripts/build.sh +``` + +Expected: `** BUILD SUCCEEDED **`. + +- [ ] **Step 4: Commit** + +```bash +git add Flipcash/Core/Session/SessionAuthenticator.swift +git commit -m "chore(session): log login attempts at info so release reports show the count" +``` + +--- + +### Task 7: Final check and handoff + +**Files:** none + +- [ ] **Step 1: Run the regression suite one last time on the finished branch** + +```bash +./Scripts/test.sh FlipcashTests/Regression_6a4fde3 +``` + +Expected: `** TEST SUCCEEDED **`, four tests. + +- [ ] **Step 2: Review the branch diff** + +```bash +git diff main...HEAD --stat +``` + +Expected files, and only these: + +``` + .claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md + .claude/plans/2026-09-09-bugsnag-6a4fde3.md + Flipcash/Core/Container.swift + Flipcash/Core/Controllers/Database/Database+Conversations.swift + Flipcash/Core/Controllers/Database/Database.swift + Flipcash/Core/Controllers/Database/DatabaseStore.swift + Flipcash/Core/Session/SessionAuthenticator.swift + FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift +``` + +- [ ] **Step 3: Report to the user, do not open a PR** + +Report in chat: the four regression tests, the fact that the crash-layer test was observed failing with `database is locked (code: 5)` before Task 2, and the two things this branch does **not** do: + +1. It does not explain the five `completeLogin` calls on one launch. Task 6 makes the next occurrence show the attempt count in Bugsnag. +2. It does not stop a stale `SessionContainer`'s in-flight writes. `logout()` (`SessionAuthenticator.swift:440-460`) still never stops `historyController`, and `completeLogin` never tears down the previous container. With one shared `Database` those writes now land on the live store, which is the pre-existing defect made visible rather than a new one. + +Ask the user to run the full `AllTargets` plan before a PR. If they want the PR opened, use `gh pr create --assignee @me` with a conventional-commit title (`fix(database): share one Database per owner across repeat logins`) and a body written with the `chrisbanes-skills:grounded-writing` skill, no attribution footer, no "Verification" section. + +--- + +## Self-review + +**Spec coverage** against the brief's Proposed direction and Verification sections: + +| Brief item | Task | +|---|---| +| One `Database` per owner keyed on `Container`, `initializeDatabase` returns the cached instance | 4, 5 | +| No explicit `close()`; ARC teardown | 4 (note in Step 3) | +| `busyTimeout = 2` | 3 | +| `.immediate` for read-then-write transactions | 2 | +| Promote the two `debug` lines to `info` | 6 | +| Regression file at `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` on `Database.makeTemp()` | 1 | +| Force contention with a held `BEGIN IMMEDIATE`, observe `SQLITE_BUSY` on unfixed code | 1 Step 2 | +| Post-fix identity assertion (`===`) across two logins for one owner | 4 | +| Risk: stale container writes land on the live store | 7 Step 3 | + +One deviation from the brief: it suggested dropping the loser's `busyTimeout` to `0.05` and asserting the throw. That test would stay red after the fix too (an immediate transaction still times out if the rival never releases), so Task 1 instead has the rival release after 200 ms, which is red on deferred and green on immediate. + +**Placeholder scan:** every code step shows the full code; every run step names the command and the expected terminal line. The one "may differ" allowance is a line number in a grep result in Task 5 Step 4, and the file-only requirement there is exact. + +**Type consistency:** `DatabaseStore.database(for:)` is the name used in Task 4's tests, Task 4's implementation, and Task 5's call site. `Container.databaseStore` is the property name in Task 5 Steps 1 and 2. `Database.makeTemp()` / `Database.removeTemp(at:)` match `FlipcashTests/TestSupport/Database+TestSupport.swift:14,21`. `Task.delay(milliseconds:)` exists at `FlipcashCore/Sources/FlipcashCore/Extensions/Task+Delay.swift:16`. `ConversationID.test(_:)` is at `FlipcashTests/TestSupport/Conversation+TestSupport.swift:11`. `Database.getConversations()` is at `Database+Conversations.swift:53`. `URL.versionFile(owner:)` is at `Database.swift:135`. `KeyPair.generate()` returns an optional (`KeyPair.swift:33`), hence the force unwrap in the test helper. diff --git a/.claude/plans/2026-09-09-bugsnag-6a4fde3.md b/.claude/plans/2026-09-09-bugsnag-6a4fde3.md new file mode 100644 index 000000000..6e298aa23 --- /dev/null +++ b/.claude/plans/2026-09-09-bugsnag-6a4fde3.md @@ -0,0 +1,49 @@ +# 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`. From b7e7c9a4dcb0e2b42c3192fbdf02a0c6d5746d73 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 17:51:07 -0400 Subject: [PATCH 2/8] test(database): reproduce replace-feed SQLITE_BUSY against a rival writer (6a4fde3) --- .../Regression_6a4fde33e96556123eb1f0ec.swift | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift diff --git a/FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift b/FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift new file mode 100644 index 000000000..f09621dbe --- /dev/null +++ b/FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift @@ -0,0 +1,51 @@ +// +// Regression_6a4fde33e96556123eb1f0ec.swift +// FlipcashTests +// +// "Failed to persist conversation state [replace-feed]" — database is locked (code: 5). +// One cold launch ran completeLogin five times for the same owner; each built a +// SessionContainer with its own Database, so four writer Connections shared one +// SQLite file. replaceConversationFeed ran a DEFERRED transaction that read before +// it wrote: once a rival writer held the lock, the snapshot upgrade returned +// SQLITE_BUSY immediately, bypassing the busy handler. +// +// Fix: Container.databaseStore hands out one Database per owner, so repeat logins +// share a single writer; read-then-write transactions take the write lock up front +// with BEGIN IMMEDIATE; busyTimeout is 2 seconds rather than 2000. +// + +import Foundation +import Testing +import FlipcashCore +@testable import Flipcash + +@MainActor +@Suite("Regression: 6a4fde3 – replace-feed fails SQLITE_BUSY against a rival writer", .bug("6a4fde33e96556123eb1f0ec")) +struct Regression_6a4fde3 { + + private func conversation(_ byte: UInt8) -> Conversation { + Conversation(id: .test(byte), members: [], lastMessage: nil, lastActivity: Date(timeIntervalSince1970: 100)) + } + + @Test("replace-feed waits for a rival writer to commit instead of failing the snapshot upgrade") + func replaceFeed_rivalWriterHoldsLock_waitsThenCommits() async throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + // Seed a row so the second feed has something to delete — the read-then-write path. + try database.replaceConversationFeed([conversation(1)], type: .contactDm) + + // A second Database on the same file is exactly what each extra SessionContainer opened. + let rival = try Database(url: url) + try rival.writer.run("BEGIN IMMEDIATE TRANSACTION") + let release = Task.detached { + try await Task.delay(milliseconds: 200) + try rival.writer.run("COMMIT TRANSACTION") + } + + try database.replaceConversationFeed([conversation(2)], type: .contactDm) + try await release.value + + let ids = try database.getConversations().map(\.id) + #expect(ids == [.test(2)]) + } +} From 68a831e082e9661b6a65d9011d381fb24fdce614 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 17:53:09 -0400 Subject: [PATCH 3/8] fix(database): take the write lock up front in read-then-write transactions --- .../Controllers/Database/Database+Conversations.swift | 8 ++++++-- Flipcash/Core/Controllers/Database/Database.swift | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Flipcash/Core/Controllers/Database/Database+Conversations.swift b/Flipcash/Core/Controllers/Database/Database+Conversations.swift index 42a5bfce6..7337fc4a6 100644 --- a/Flipcash/Core/Controllers/Database/Database+Conversations.swift +++ b/Flipcash/Core/Controllers/Database/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). 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) } diff --git a/Flipcash/Core/Controllers/Database/Database.swift b/Flipcash/Core/Controllers/Database/Database.swift index 5b3419ee8..3e3732e3d 100644 --- a/Flipcash/Core/Controllers/Database/Database.swift +++ b/Flipcash/Core/Controllers/Database/Database.swift @@ -53,7 +53,8 @@ nonisolated class Database: @unchecked Sendable { func transaction(silent: Bool = false, _ block: (Database) throws -> Void) rethrows { do { let startChangeCount = writer.totalChanges - try writer.transaction { [unowned self] in + // IMMEDIATE: callers read and write inside the block; see replaceConversationFeed. + try writer.transaction(.immediate) { [unowned self] in try block(self) } let endChangeCount = writer.totalChanges From b3944d57772b571553119665cb1c74fa90c9a472 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 17:54:59 -0400 Subject: [PATCH 4/8] fix(database): busy timeout is in seconds, arm 2s not 33min --- Flipcash/Core/Controllers/Database/Database.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Flipcash/Core/Controllers/Database/Database.swift b/Flipcash/Core/Controllers/Database/Database.swift index 3e3732e3d..58a05179e 100644 --- a/Flipcash/Core/Controllers/Database/Database.swift +++ b/Flipcash/Core/Controllers/Database/Database.swift @@ -33,13 +33,13 @@ nonisolated class Database: @unchecked Sendable { self.writer = try Connection(url.path) - writer.busyTimeout = 2000 // 2 sec + writer.busyTimeout = 2 // seconds try writer.run("PRAGMA journal_mode = WAL;") try writer.run("PRAGMA cache_size = 10000;") try writer.run("PRAGMA foreign_keys = ON;") self.reader = try Connection(url.path, readonly: true) - reader.busyTimeout = 2000 // 2 Sec + reader.busyTimeout = 2 // seconds try createTablesIfNeeded() } From 39987acbc185ce80f37ea3b1d4375a4cc6962268 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 17:54:59 -0400 Subject: [PATCH 5/8] feat(database): DatabaseStore caches one Database per owner --- .../Controllers/Database/DatabaseStore.swift | 49 +++++++++++++++++++ .../Regression_6a4fde33e96556123eb1f0ec.swift | 39 +++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 Flipcash/Core/Controllers/Database/DatabaseStore.swift diff --git a/Flipcash/Core/Controllers/Database/DatabaseStore.swift b/Flipcash/Core/Controllers/Database/DatabaseStore.swift new file mode 100644 index 000000000..d96cf2ee8 --- /dev/null +++ b/Flipcash/Core/Controllers/Database/DatabaseStore.swift @@ -0,0 +1,49 @@ +// +// DatabaseStore.swift +// Flipcash +// + +import Foundation +import FlipcashCore + +private let logger = Logger(label: "flipcash.database-store") + +/// Opens one `Database` per owner and returns that same instance on every later request, +/// so repeat logins in one process share a single SQLite writer instead of contending for the file. +final class DatabaseStore { + + private var databases: [PublicKey: Database] = [:] + + /// The owner's `Database`, opened (and rebuilt if its on-disk version is outdated) on first use. + func database(for owner: PublicKey) throws -> Database { + if let database = databases[owner] { + return database + } + + try createApplicationSupportIfNeeded() + + // 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(owner: owner)) ?? 0 + let currentVersion = try InfoPlist.value(for: "SQLiteVersion").integer() + if currentVersion > userVersion { + try Database.deleteStore(owner: owner) + logger.error("Outdated user version, deleted database.") + try Database.setUserVersion(version: currentVersion, owner: owner) + } + + let database = try Database(url: .dataStore(owner: owner)) + databases[owner] = database + return database + } + + private func createApplicationSupportIfNeeded() throws { + if !FileManager.default.fileExists(atPath: URL.applicationSupportDirectory.path) { + try FileManager.default.createDirectory( + at: .applicationSupportDirectory, + withIntermediateDirectories: false + ) + } + } +} diff --git a/FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift b/FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift index f09621dbe..25e4135c2 100644 --- a/FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift +++ b/FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift @@ -48,4 +48,43 @@ struct Regression_6a4fde3 { let ids = try database.getConversations().map(\.id) #expect(ids == [.test(2)]) } + + @Test("busy timeout is armed in seconds, not milliseconds") + func busyTimeout_isTwoSeconds() throws { + let (database, url) = try Database.makeTemp() + defer { Database.removeTemp(at: url) } + #expect(database.writer.busyTimeout == 2) + #expect(database.reader.busyTimeout == 2) + } + + @Test("the store hands the same Database back for the same owner") + func databaseStore_sameOwnerTwice_returnsOneInstance() throws { + try withThrowawayOwner { owner in + let store = DatabaseStore() + let first = try store.database(for: owner) + let second = try store.database(for: owner) + #expect(first === second) + } + } + + @Test("the store keeps different owners apart") + func databaseStore_twoOwners_returnsDistinctInstances() throws { + try withThrowawayOwner { alice in + try withThrowawayOwner { bob in + let store = DatabaseStore() + let alicesDatabase = try store.database(for: alice) + let bobsDatabase = try store.database(for: bob) + #expect(alicesDatabase !== bobsDatabase) + } + } + } + + private func withThrowawayOwner(_ body: (PublicKey) throws -> Void) throws { + let owner = KeyPair.generate()!.publicKey + defer { + try? Database.deleteStore(owner: owner) + try? FileManager.default.removeItem(at: .versionFile(owner: owner)) + } + try body(owner) + } } From 739c8a1fc66174aafdf4f2f79ea63c430719f8d5 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 17:56:11 -0400 Subject: [PATCH 6/8] fix(session): share one Database per owner across repeat logins (6a4fde3) --- Flipcash/Core/Container.swift | 2 ++ .../Core/Session/SessionAuthenticator.swift | 30 +------------------ 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/Flipcash/Core/Container.swift b/Flipcash/Core/Container.swift index 61bba8652..422aa208f 100644 --- a/Flipcash/Core/Container.swift +++ b/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 } diff --git a/Flipcash/Core/Session/SessionAuthenticator.swift b/Flipcash/Core/Session/SessionAuthenticator.swift index 17114cb37..0a931f840 100644 --- a/Flipcash/Core/Session/SessionAuthenticator.swift +++ b/Flipcash/Core/Session/SessionAuthenticator.swift @@ -223,7 +223,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, @@ -290,34 +290,6 @@ final class SessionAuthenticator { ) } - // MARK: - Database - - - private func initializeDatabase(owner: PublicKey) throws -> Database { - try createApplicationSupportIfNeeded() - - // 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(owner: owner)) ?? 0 - let currentVersion = try InfoPlist.value(for: "SQLiteVersion").integer() - if currentVersion > userVersion { - try Database.deleteStore(owner: owner) - logger.error("Outdated user version, deleted database.") - try Database.setUserVersion(version: currentVersion, owner: owner) - } - - return try Database(url: .dataStore(owner: owner)) - } - - private func createApplicationSupportIfNeeded() throws { - if !FileManager.default.fileExists(atPath: URL.applicationSupportDirectory.path) { - try FileManager.default.createDirectory( - at: .applicationSupportDirectory, - withIntermediateDirectories: false - ) - } - } - // MARK: - Login - func initialize(using mnemonic: MnemonicPhrase, isRegistration: Bool) async throws -> InitializedAccount { From f49ed71b02323bebc023bb8953d9a7c17b2f8a82 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 17:56:12 -0400 Subject: [PATCH 7/8] chore(session): log login attempts at info so release reports show the count --- Flipcash/Core/Session/SessionAuthenticator.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Flipcash/Core/Session/SessionAuthenticator.swift b/Flipcash/Core/Session/SessionAuthenticator.swift index 0a931f840..30f5d1099 100644 --- a/Flipcash/Core/Session/SessionAuthenticator.swift +++ b/Flipcash/Core/Session/SessionAuthenticator.swift @@ -125,7 +125,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 { @@ -356,7 +356,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, From 85f8c1fd1ede54fa0c07f76fe29892623da3f23a Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Wed, 9 Sep 2026 17:56:31 -0400 Subject: [PATCH 8/8] docs(plans): mark the 6a4fde3 plan executed and record run notes --- .../plans/2026-09-09-bugsnag-6a4fde3-plan.md | 81 ++++++++++--------- 1 file changed, 45 insertions(+), 36 deletions(-) diff --git a/.claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md b/.claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md index 3aa70833f..19d21a31f 100644 --- a/.claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md +++ b/.claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md @@ -51,7 +51,7 @@ A single test: **Files:** none -- [ ] **Step 1: Create the fix branch from the current HEAD** +- [x] **Step 1: Create the fix branch from the current HEAD** The worktree sits on `claude/flipcash-ios-error-triage-7e50ad` at `8042ff9d`, which is `main`. The user's rules forbid `claude/`-prefixed branch names for the work itself. @@ -61,7 +61,7 @@ git checkout -b fix/database-per-owner Expected: `Switched to a new branch 'fix/database-per-owner'` -- [ ] **Step 2: Confirm the tree is clean apart from the two plan files** +- [x] **Step 2: Confirm the tree is clean apart from the two plan files** ```bash git status --short @@ -74,7 +74,7 @@ Expected: ?? .claude/plans/2026-09-09-bugsnag-6a4fde3.md ``` -- [ ] **Step 3: Commit the triage brief and this plan** +- [x] **Step 3: Commit the triage brief and this plan** ```bash git add .claude/plans/2026-09-09-bugsnag-6a4fde3.md .claude/plans/2026-09-09-bugsnag-6a4fde3-plan.md @@ -92,7 +92,7 @@ Why it discriminates: on unfixed code the transaction is `BEGIN DEFERRED`. The ` **Files:** - Create: `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```swift // @@ -148,7 +148,7 @@ struct Regression_6a4fde3 { } ``` -- [ ] **Step 2: Run it and watch it fail at the crash layer** +- [x] **Step 2: Run it and watch it fail at the crash layer** ```bash ./Scripts/test.sh FlipcashTests/Regression_6a4fde3/replaceFeed_rivalWriterHoldsLock_waitsThenCommits @@ -156,7 +156,7 @@ struct Regression_6a4fde3 { Expected: `** TEST FAILED **`. The failure for `replaceFeed_rivalWriterHoldsLock_waitsThenCommits` must be a thrown error whose text contains `database is locked` and `(code: 5)`, the production signature. SQLite.swift includes the failing statement in the description when it has one, so the local text will read `database is locked (DELETE FROM "conversations" ...) (code: 5)` where production showed only `database is locked (code: 5)`; the shared part is what matters. If the test instead fails on the `#expect`, or passes, stop: the reproduction is not hitting the deferred-snapshot path and the test needs rethinking, not the fix. -- [ ] **Step 3: Commit the red test** +- [x] **Step 3: Commit the red test** ```bash git add FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift @@ -174,7 +174,7 @@ git commit -m "test(database): reproduce replace-feed SQLITE_BUSY against a riva Three sites read inside the transaction before writing: `replaceConversationFeed` (the `SELECT` of doomed ids), `persistMessages` (the `pluck` of the current cursor), and every caller of the `Database.transaction` helper (`Database+Balance.swift:158`, `:203`, `Database+Rates.swift:45`, `Database+VerifiedProtos.swift:21`, `:56` all read rows before upserting). The remaining `writer.transaction {` sites begin with a `DELETE` or `INSERT`, which takes the write lock as its first statement and so already consults the busy handler; leave them alone. -- [ ] **Step 1: Make `replaceConversationFeed` immediate** +- [x] **Step 1: Make `replaceConversationFeed` immediate** In `Database+Conversations.swift`, change line 230 from: @@ -191,7 +191,7 @@ to: try writer.transaction(.immediate) { ``` -- [ ] **Step 2: Make `persistMessages` immediate** +- [x] **Step 2: Make `persistMessages` immediate** In `Database+Conversations.swift`, change line 284 (inside `persistMessages`) from: @@ -205,7 +205,7 @@ to: try writer.transaction(.immediate) { ``` -- [ ] **Step 3: Make the `Database.transaction` helper immediate** +- [x] **Step 3: Make the `Database.transaction` helper immediate** In `Database.swift`, change line 56 from: @@ -219,7 +219,7 @@ to: try writer.transaction(.immediate) { [unowned self] in ``` -- [ ] **Step 4: Run the regression test and see it pass** +- [x] **Step 4: Run the regression test and see it pass** ```bash ./Scripts/test.sh FlipcashTests/Regression_6a4fde3/replaceFeed_rivalWriterHoldsLock_waitsThenCommits @@ -227,7 +227,7 @@ to: Expected: `** TEST SUCCEEDED **`. The test now takes a little over 200 ms because `BEGIN IMMEDIATE` waits for the rival's commit. -- [ ] **Step 5: Run the existing database and conversation suites to catch a regression in the helper change** +- [x] **Step 5: Run the existing database and conversation suites to catch a regression in the helper change** ```bash ./Scripts/test.sh FlipcashTests/DatabaseBalanceUpsertTests FlipcashTests/DatabaseLiveSupplyTests FlipcashTests/ConversationControllerTests @@ -237,7 +237,7 @@ Those are the struct names at `FlipcashTests/Database/Database+BalanceUpsertTest Expected: `** TEST SUCCEEDED **`. -- [ ] **Step 6: Commit** +- [x] **Step 6: Commit** ```bash git add Flipcash/Core/Controllers/Database/Database+Conversations.swift Flipcash/Core/Controllers/Database/Database.swift @@ -254,7 +254,7 @@ git commit -m "fix(database): take the write lock up front in read-then-write tr - Modify: `Flipcash/Core/Controllers/Database/Database.swift:36` and `:42` - Test: `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` -- [ ] **Step 1: Add the failing test** +- [x] **Step 1: Add the failing test** Append inside `struct Regression_6a4fde3`, after `replaceFeed_rivalWriterHoldsLock_waitsThenCommits`: @@ -270,7 +270,7 @@ Append inside `struct Regression_6a4fde3`, after `replaceFeed_rivalWriterHoldsLo } ``` -- [ ] **Step 2: Run it and see it fail** +- [x] **Step 2: Run it and see it fail** ```bash ./Scripts/test.sh FlipcashTests/Regression_6a4fde3/busyTimeout_isTwoSeconds @@ -278,7 +278,7 @@ Append inside `struct Regression_6a4fde3`, after `replaceFeed_rivalWriterHoldsLo Expected: `** TEST FAILED **` with `Expectation failed: (database.writer.busyTimeout → 2000.0) == 2`. -- [ ] **Step 3: Fix the two assignments** +- [x] **Step 3: Fix the two assignments** In `Database.swift`, replace lines 36 and 42: @@ -304,7 +304,7 @@ becomes reader.busyTimeout = 2 // seconds ``` -- [ ] **Step 4: Run the whole regression suite and see it pass** +- [x] **Step 4: Run the whole regression suite and see it pass** ```bash ./Scripts/test.sh FlipcashTests/Regression_6a4fde3 @@ -312,7 +312,7 @@ becomes Expected: `** TEST SUCCEEDED **`, two tests passing. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add Flipcash/Core/Controllers/Database/Database.swift FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift @@ -329,7 +329,7 @@ This is the root-cause fix. The identity test is written against `DatabaseStore` - Create: `Flipcash/Core/Controllers/Database/DatabaseStore.swift` - Test: `FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift` -- [ ] **Step 1: Add the failing tests** +- [x] **Step 1: Add the failing tests** Append inside `struct Regression_6a4fde3`, after `busyTimeout_isTwoSeconds`: @@ -372,7 +372,7 @@ Append inside `struct Regression_6a4fde3`, after `busyTimeout_isTwoSeconds`: } ``` -- [ ] **Step 2: Run the suite and see it fail to compile** +- [x] **Step 2: Run the suite and see it fail to compile** ```bash ./Scripts/test.sh FlipcashTests/Regression_6a4fde3 @@ -380,7 +380,7 @@ Append inside `struct Regression_6a4fde3`, after `busyTimeout_isTwoSeconds`: Expected: `** TEST FAILED **` (build failure) with `cannot find 'DatabaseStore' in scope`. This is the expected red state: the type does not exist yet. -- [ ] **Step 3: Create `DatabaseStore`** +- [x] **Step 3: Create `DatabaseStore`** Create `Flipcash/Core/Controllers/Database/DatabaseStore.swift`. The body of `database(for:)` is `SessionAuthenticator.initializeDatabase` (`SessionAuthenticator.swift:295-310`) and `createApplicationSupportIfNeeded` (`:312-319`) moved verbatim, with the cache lookup in front. @@ -441,7 +441,7 @@ Notes for the implementer: - `PublicKey` is already used as a dictionary key elsewhere (`TokenCardStack.swift:83`), so it is `Hashable`. - There is deliberately no `close()` or eviction. SQLite.swift's `Connection` closes only in `deinit`, and `HistoryController.sync()` (`HistoryController.swift:105`) holds the database in a `Task` with no `[weak self]`, so an explicit close would race in-flight work. A cached `Database` lives for the process; the cost is two open connections per owner ever logged in. -- [ ] **Step 4: Run the suite and see it pass** +- [x] **Step 4: Run the suite and see it pass** ```bash ./Scripts/test.sh FlipcashTests/Regression_6a4fde3 @@ -449,7 +449,7 @@ Notes for the implementer: Expected: `** TEST SUCCEEDED **`, four tests passing. -- [ ] **Step 5: Commit** +- [x] **Step 5: Commit** ```bash git add Flipcash/Core/Controllers/Database/DatabaseStore.swift FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift @@ -464,7 +464,7 @@ git commit -m "feat(database): DatabaseStore caches one Database per owner" - Modify: `Flipcash/Core/Container.swift:16-21`, `:35-40` - Modify: `Flipcash/Core/Session/SessionAuthenticator.swift:226`, `:293-319` -- [ ] **Step 1: Hold the store on `Container`** +- [x] **Step 1: Hold the store on `Container`** In `Container.swift`, add a property after `let notificationController: NotificationController` (line 21): @@ -480,7 +480,7 @@ and initialise it in `init()` after `self.notificationController = NotificationC self.databaseStore = DatabaseStore() ``` -- [ ] **Step 2: Ask the store in `createSessionContainer`** +- [x] **Step 2: Ask the store in `createSessionContainer`** In `SessionAuthenticator.swift`, change line 226 from: @@ -494,7 +494,7 @@ to: let database = try! container.databaseStore.database(for: ownerPublicKey) ``` -- [ ] **Step 3: Delete the moved code** +- [x] **Step 3: Delete the moved code** In `SessionAuthenticator.swift`, delete lines 293 to 320 in full, that is the `// MARK: - Database -` header, `initializeDatabase(owner:)`, and `createApplicationSupportIfNeeded()`: @@ -531,7 +531,7 @@ In `SessionAuthenticator.swift`, delete lines 293 to 320 in full, that is the `/ Leave the `// MARK: - Login -` header that follows in place. -- [ ] **Step 4: Verify the store is now the only `Database` constructor in the app target** +- [x] **Step 4: Verify the store is now the only `Database` constructor in the app target** ```bash grep -rn "Database(url" Flipcash --include=*.swift @@ -545,7 +545,7 @@ Flipcash/Core/Controllers/Database/DatabaseStore.swift:37: let database = (The line number may differ by one or two; the file must be the only match.) -- [ ] **Step 5: Build the app** +- [x] **Step 5: Build the app** ```bash ./Scripts/build.sh @@ -553,7 +553,7 @@ Flipcash/Core/Controllers/Database/DatabaseStore.swift:37: let database = Expected: `** BUILD SUCCEEDED **`. -- [ ] **Step 6: Run the regression suite plus the suites that construct a `Container`** +- [x] **Step 6: Run the regression suite plus the suites that construct a `Container`** ```bash ./Scripts/test.sh FlipcashTests/Regression_6a4fde3 FlipcashTests/DeepLinkControllerTests @@ -563,7 +563,7 @@ Expected: `** BUILD SUCCEEDED **`. Expected: `** TEST SUCCEEDED **`. -- [ ] **Step 7: Commit** +- [x] **Step 7: Commit** ```bash git add Flipcash/Core/Container.swift Flipcash/Core/Session/SessionAuthenticator.swift @@ -579,7 +579,7 @@ Release builds bootstrap logging at `.info` (`FlipcashCore/Sources/FlipcashCore/ **Files:** - Modify: `Flipcash/Core/Session/SessionAuthenticator.swift:128`, `:387` -- [ ] **Step 1: Log each `initializeState` attempt at `info` with its retry count** +- [x] **Step 1: Log each `initializeState` attempt at `info` with its retry count** Change line 128 from: @@ -593,7 +593,7 @@ to: logger.info("initializeState called", metadata: ["count": "\(count)"]) ``` -- [ ] **Step 2: Log `completeLogin` at `info`** +- [x] **Step 2: Log `completeLogin` at `info`** Change line 387 from: @@ -607,7 +607,7 @@ to: logger.info("completeLogin", metadata: ["owner": "\(initializedAccount.keyAccount.ownerPublicKey)"]) ``` -- [ ] **Step 3: Build** +- [x] **Step 3: Build** ```bash ./Scripts/build.sh @@ -615,7 +615,7 @@ to: Expected: `** BUILD SUCCEEDED **`. -- [ ] **Step 4: Commit** +- [x] **Step 4: Commit** ```bash git add Flipcash/Core/Session/SessionAuthenticator.swift @@ -628,7 +628,7 @@ git commit -m "chore(session): log login attempts at info so release reports sho **Files:** none -- [ ] **Step 1: Run the regression suite one last time on the finished branch** +- [x] **Step 1: Run the regression suite one last time on the finished branch** ```bash ./Scripts/test.sh FlipcashTests/Regression_6a4fde3 @@ -636,7 +636,7 @@ git commit -m "chore(session): log login attempts at info so release reports sho Expected: `** TEST SUCCEEDED **`, four tests. -- [ ] **Step 2: Review the branch diff** +- [x] **Step 2: Review the branch diff** ```bash git diff main...HEAD --stat @@ -655,7 +655,7 @@ Expected files, and only these: FlipcashTests/Regressions/Regression_6a4fde33e96556123eb1f0ec.swift ``` -- [ ] **Step 3: Report to the user, do not open a PR** +- [x] **Step 3: Report to the user, do not open a PR** Report in chat: the four regression tests, the fact that the crash-layer test was observed failing with `database is locked (code: 5)` before Task 2, and the two things this branch does **not** do: @@ -687,3 +687,12 @@ One deviation from the brief: it suggested dropping the loser's `busyTimeout` to **Placeholder scan:** every code step shows the full code; every run step names the command and the expected terminal line. The one "may differ" allowance is a line number in a grep result in Task 5 Step 4, and the file-only requirement there is exact. **Type consistency:** `DatabaseStore.database(for:)` is the name used in Task 4's tests, Task 4's implementation, and Task 5's call site. `Container.databaseStore` is the property name in Task 5 Steps 1 and 2. `Database.makeTemp()` / `Database.removeTemp(at:)` match `FlipcashTests/TestSupport/Database+TestSupport.swift:14,21`. `Task.delay(milliseconds:)` exists at `FlipcashCore/Sources/FlipcashCore/Extensions/Task+Delay.swift:16`. `ConversationID.test(_:)` is at `FlipcashTests/TestSupport/Conversation+TestSupport.swift:11`. `Database.getConversations()` is at `Database+Conversations.swift:53`. `URL.versionFile(owner:)` is at `Database.swift:135`. `KeyPair.generate()` returns an optional (`KeyPair.swift:33`), hence the force unwrap in the test helper. + +--- + +## Execution notes (2026-09-09) + +- `./Scripts/test.sh //` selected 0 tests and still printed `** TEST SUCCEEDED **`; every run used the suite form `FlipcashTests/Regression_6a4fde3`. +- Red run on unfixed code failed in 0.009 s with `Caught error: database is locked (code: 5)` — the deferred snapshot upgrade returns SQLITE_BUSY without waiting. Green after `.immediate` took 0.26 s (the rival's 200 ms hold). +- `#expect(try a !== b)` does not compile ("errors thrown from here are not handled"); the two lookups are hoisted into locals. +- The build's `gengoogle` phase rewrites `Flipcash/Supporting Files/GoogleService-Info.plist`; it is on the pre-commit blocklist and was never staged.