Skip to content

fix(ios): do not treat WAL sidecars as dual databases - #758

Merged
9thLevelSoftware merged 1 commit into
mainfrom
fix/ios-dual-databases-wal-sidecars
Sep 4, 2026
Merged

fix(ios): do not treat WAL sidecars as dual databases#758
9thLevelSoftware merged 1 commit into
mainfrom
fix/ios-dual-databases-wal-sidecars

Conversation

@9thLevelSoftware

Copy link
Copy Markdown
Owner

Fixes #757

Summary

TestFlight 1.0.2 (2026090419) shows non-retryable DB_DUAL_DATABASES even though PR #755 is in that build.

migrateLegacyLibraryRootIfNeeded() threw whenever SQLiter had vitruvian.db WAL sidecars and Library/vitruvian.db was absent. That Library path was never the live SQLiter location. The coordinator never got to migrate vitruvian.dbphoenix.db.

What changed

  • Return (log only) when the Library-root main file is missing.
  • Still throw when both main files exist.
  • Still throw when a Library main would be copied onto leftover SQLiter sidecars.
  • Do not delete orphan sidecars.

Verification

  • RED then GREEN: Issue725RecurrenceTest new cases (SQLiter legacy+WAL, orphan sidecars, true dual mains).
  • GREEN: existing Issue725RecurrenceTest (9 tests, 0 failures, parent --rerun-tasks).
  • GREEN: Issue725FreshInstallDriverFactoryTest (2 tests, 0 failures).
  • GREEN: DatabaseFileMigrationCoordinatorTest (23 tests, 0 failures).
  • GREEN: :shared:compileKotlinIosArm64.

Android file-migration paths are untouched.

PR #755 threw DB_DUAL_DATABASES when Library/vitruvian.db was absent
but SQLiter still had vitruvian.db-wal/-shm. That is the normal
pre-cutover WAL layout, not two databases. Keep failing closed only
when both main files exist, or when a Library main would be copied
onto leftover SQLiter sidecars.
Copilot AI lite review requested due to automatic review settings September 4, 2026 21:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T21:36:42.413857Z cdd94e8 PR opened
🔒 Security Review Completed 2026-09-04T21:37:35.612265Z cdd94e8 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kilo-code-bot

kilo-code-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Code Review Roast 🔥

Verdict: No Issues Found | Recommendation: Merge

Oh wait, this PR is actually clean. I had my flamethrower warmed up and everything, ready to roast some DB_DUAL_DATABASES hysteria — and instead I found a four-line fix that turns a screaming false-positive into a quiet NSLog. I need to sit down.

The bug in PR #755 was a classic case of treating a symptom as the disease: WAL sidecars without a main file are normal SQLite post-checkpoint garbage, not a second database. This PR correctly narrows the throw to the two cases that actually matter (both mains exist, or a Library main would clobber leftover SQLiter sidecars) and lets everything else pass through. The four new tests pin each branch — including the "don't regress into over-permissiveness" guard test — which is the kind of thing senior reviewers love to see and rarely do.

The extracted legacyExists local is a small but real win: one filesystem call instead of two on the common path. The two NSLog messages are differentiated by location, which is the right call because ops people grep.

Correctness / Safety Findings
No correctness or safety findings.

Ponytail Review
Ponytail: Lean already. Ship.

Ponytail net: 0 lines.

Suggested Minimal Patch
No patch needed.

Final Merge Guidance
Can merge as-is. The fix is minimal, the tests cover all four relevant branches (SQLiter legacy + WAL, SQLiter legacy + WAL migrates, orphan SQLiter sidecars, true dual mains), and the remaining throw paths are unchanged. Optional follow-up if anyone has the appetite: one test that exercises Library-only orphan sidecars (currently the prod path logs them but is untested). Not blocking.

Files Reviewed (2 files)
  • shared/src/iosMain/kotlin/com/devil/phoenixproject/data/local/IosDatabaseFileOperations.kt — 0 issues
  • shared/src/iosTest/kotlin/com/devil/phoenixproject/data/local/Issue725RecurrenceTest.kt — 0 issues

📊 Overall: Like a bartender who actually knows when to stop pouring. PR #755 was over-served; PR #758 knows the difference between WAL residue and a real dual-database emergency.


Reviewed by minimax-m3 · Input: 35.8K · Output: 5.9K · Cached: 138.6K

Review guidance: REVIEW.md from base branch main

@9thLevelSoftware
9thLevelSoftware merged commit 607fd2f into main Sep 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: iOS DB_DUAL_DATABASES on TestFlight 2026090419 after PR #755

2 participants