fix: never merge two vaults into one folder, and recover devices that already did (#45) - #47
Conversation
A server hosting multiple vaults (one Syncthing share per vault) could get its second share assigned the same local path as the first. VaultManager collapsed every share onto the Obsidian root whenever the root was itself a vault (`baseIsVault`) or the share name matched the root, and neither the Swift path assignment nor the Go bridge checked whether that path was already taken. Syncthing then merged both vaults into one directory and pushed the mix back to both peers — silent, destructive data loss. Two layers of defense: - Go (hard floor): AcceptPendingFolder rejects a folder whose local path is already used by another configured folder (cleaned + case-insensitive, matching case-folding APFS). Two distinct folder IDs can never share a directory regardless of what the client computes. - Swift (correct mapping): resolveSharePath keeps the Obsidian root a pure container — each vault maps to its own `root/<name>` subdirectory. The collapse-into-root shortcuts apply only while the root is free; a genuine name clash is disambiguated (`<name> (2)`) instead of merged. Existing single-vault setups are untouched (the first vault still owns the root). This realizes the intended setup — pick the Obsidian root once, every vault auto-syncs into its own folder, new desktop vaults appear automatically — without ever colliding. Tests: Go TestAcceptPendingFolderPathCollision (same path / trailing slash / case variant rejected, distinct path accepted); Swift VaultManagerSharePathTests (9 cases incl. the #45 regression, dedup, case-insensitivity, sequential accept).
Add the path-collision fix to the 1.7.1 Fixed section (listed first as the more severe, data-safety fix) and bump the entry date to the release day.
…ath (#45) The A+D fix prevents new path collisions, but devices that already merged two vaults onto one local folder under 1.6.0/1.7.0 keep mixing them on every sync. Add a launch-time migration shield that stops the bleeding without touching data. - Go: SetFolderPaused(folderID, paused) pauses/resumes a folder in place (config-modify, idempotent no-op when unchanged), plus TestSetFolderPaused. - PathCollisionGuard: pure, injectable detection — group folders by the same canonical + lowercased path the accept-time guard uses; a path held by >=2 folders is a collision. A pause-once core pauses each colliding folder exactly once, recorded in a serialized UserDefaults sidecar; it never auto-resumes (a deliberate resume for recovery is not fought) and leaves a failed pause unrecorded so it retries next launch. Wired off-main after the path reconcile; clears its sidecar entry on removal. - A critical .pathCollision SyncIssueItem leads the Sync Issues list with a localized explanation and a manual-recovery remediation. No automated fix (never delete/rename/re-accept). Localized in EN/DE/ES/zh-Hans. Recovery stays manual: remove the affected vault and the A+D guard re-adds it into its own folder, unpaused. Go + 13 new Swift tests green; xcframework rebuilt with SetFolderPaused.
|
Warning Review limit reached
More reviews will be available in 42 minutes and 54 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughImplements a two-part fix for issue ChangesPath Collision Migration Shield
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/VaultSync/Services/VaultManager.swift`:
- Line 203: The logger.info call in the VaultManager.swift file contains
user-derived values folderName and path that could expose personal context or
vault names in crash aggregators or device logs. Modify the logger.info
statement to wrap folderName and path with the privacy: .private parameter to
prevent accidental exposure of these sensitive user-derived values in debug logs
and crash reports while keeping the other diagnostic values visible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3b4ad4f-de8b-4c01-922a-f5530657df1c
📒 Files selected for processing (16)
CHANGELOG.mdgo/bridge/folders.gogo/bridge/folders_test.gogo/bridge/pendingfolders.gogo/bridge/pendingfolders_test.goios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Views/SyncIssuesView.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/es.lproj/Localizable.stringsios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSyncTests/PathCollisionGuardTests.swiftios/VaultSyncTests/VaultManagerSharePathTests.swift
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🧰 Additional context used
📓 Path-based instructions (8)
**/*.swift
📄 CodeRabbit inference engine (Custom checks)
For Swift background execution changes, pass if work is bounded, cancellation-aware, handles expiration callbacks, and records errors without leaking private vault data. Fail only when background work can continue unbounded, miss cleanup, or violate iOS background execution constraints.
Files:
ios/VaultSync/Services/SyncBridgeService.swiftios/VaultSyncTests/VaultManagerSharePathTests.swiftios/VaultSync/Views/SyncIssuesView.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSyncTests/PathCollisionGuardTests.swiftios/VaultSync/Services/SyncthingManager.swift
ios/**/*.swift
📄 CodeRabbit inference engine (README.md)
ios/**/*.swift: Use Swift 6 and SwiftUI for iOS app development
Implement VoiceOver and Dynamic Type accessibility support throughout the app
UseBGAppRefreshTaskandBGContinuedProcessingTask(iOS 26+ when available) for background sync operations
Use APNs silent push notifications via Cloud Relay for server-to-iPhone wake-ups
Implement side-by-side diff resolution for Markdown file conflicts
Provide an activity timeline and diagnostics interface showing exactly what synced and when
Implement QR code pairing for Syncthing Device ID connection setup
Detect and list available Obsidian vaults automatically upon connection
Files:
ios/VaultSync/Services/SyncBridgeService.swiftios/VaultSyncTests/VaultManagerSharePathTests.swiftios/VaultSync/Views/SyncIssuesView.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSyncTests/PathCollisionGuardTests.swiftios/VaultSync/Services/SyncthingManager.swift
⚙️ CodeRabbit configuration file
ios/**/*.swift: Focus on Swift 6 strict concurrency, Sendable/MainActor correctness, Task cancellation,
retain cycles, memory pressure, SwiftUI observation state, StoreKit/APNs flows, and iOS background execution limits.
Pay special attention to BGAppRefreshTask and BGContinuedProcessingTask behavior, expiration handling,
bounded work, and cleanup when the app is suspended or terminated.
Files:
ios/VaultSync/Services/SyncBridgeService.swiftios/VaultSyncTests/VaultManagerSharePathTests.swiftios/VaultSync/Views/SyncIssuesView.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSyncTests/PathCollisionGuardTests.swiftios/VaultSync/Services/SyncthingManager.swift
ios/**/*.{swift,pbxproj}
📄 CodeRabbit inference engine (README.md)
Target iOS / iPadOS 18 or later as the minimum deployment target
Files:
ios/VaultSync/Services/SyncBridgeService.swiftios/VaultSyncTests/VaultManagerSharePathTests.swiftios/VaultSync/Views/SyncIssuesView.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSyncTests/PathCollisionGuardTests.swiftios/VaultSync/Services/SyncthingManager.swift
ios/**/*.{swift,strings,stringsdict}
📄 CodeRabbit inference engine (README.md)
Support localization in English, German, Spanish, and Simplified Chinese
Files:
ios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSync/es.lproj/Localizable.stringsios/VaultSync/en.lproj/Localizable.stringsios/VaultSyncTests/VaultManagerSharePathTests.swiftios/VaultSync/Views/SyncIssuesView.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSyncTests/PathCollisionGuardTests.swiftios/VaultSync/Services/SyncthingManager.swift
**/*
⚙️ CodeRabbit configuration file
**/*: VaultSync syncs private Obsidian notes through Syncthing. Treat data loss,
privacy leaks, security regressions, and broken sync behavior as high priority.
Do not nitpick formatting unless it affects maintainability, correctness, or public API clarity.
Flag any accidental logging, telemetry, crash reporting, or network transfer of note contents,
vault paths, filenames with private context, API keys, APNs tokens, relay keys, or security-scoped bookmark data.
Files:
ios/VaultSync/Services/SyncBridgeService.swiftgo/bridge/folders.goCHANGELOG.mdios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/zh-Hans.lproj/Localizable.stringsgo/bridge/pendingfolders_test.goios/VaultSync/es.lproj/Localizable.stringsios/VaultSync/en.lproj/Localizable.stringsios/VaultSyncTests/VaultManagerSharePathTests.swiftgo/bridge/pendingfolders.goios/VaultSync/Views/SyncIssuesView.swiftgo/bridge/folders_test.goios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSyncTests/PathCollisionGuardTests.swiftios/VaultSync/Services/SyncthingManager.swift
**
⚙️ CodeRabbit configuration file
**:
VaultSync
Self-hosted Obsidian vault sync for iPhone and iPad.
Your notes sync peer-to-peer over Syncthing, straight into Obsidian's iOS sandbox — no note cloud, no account, no tracking.
🔭 Why VaultSync
- Peer-to-peer & private — syncs directly between your own devices over Syncthing. No note cloud, no account, no tracking.
- Lands in Obsidian — files sync into Obsidian's iOS sandbox, where the app already looks for them.
- Pair by QR, resolve conflicts — connect your server in seconds; settle Markdown conflicts with side-by-side diffs.
- Server changes wake your iPhone — optional Cloud Relay nudges the app the moment your server updates, so incoming notes land eve...
Files:
ios/VaultSync/Services/SyncBridgeService.swiftgo/bridge/folders.goCHANGELOG.mdios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/zh-Hans.lproj/Localizable.stringsgo/bridge/pendingfolders_test.goios/VaultSync/es.lproj/Localizable.stringsios/VaultSync/en.lproj/Localizable.stringsios/VaultSyncTests/VaultManagerSharePathTests.swiftgo/bridge/pendingfolders.goios/VaultSync/Views/SyncIssuesView.swiftgo/bridge/folders_test.goios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSyncTests/PathCollisionGuardTests.swiftios/VaultSync/Services/SyncthingManager.swift
go/bridge/**/*.go
⚙️ CodeRabbit configuration file
go/bridge/**/*.go: This code crosses the gomobile Swift-Go boundary. Verify exported signatures use only gomobile-safe primitive types,
preserve the JSON string contract, keep empty-string success conventions intact, and avoid breaking Swift decoding tests.
Review Syncthing lifecycle, locking, error strings, and noassets build assumptions carefully.
Files:
go/bridge/folders.gogo/bridge/pendingfolders_test.gogo/bridge/pendingfolders.gogo/bridge/folders_test.go
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Review public documentation for technical accuracy, privacy/security claims, App Store-facing wording,
setup correctness, and consistency with the free app plus optional Cloud Relay subscription model.
Files:
CHANGELOG.md
🧠 Learnings (1)
📚 Learning: 2026-06-10T18:47:10.724Z
Learnt from: psimaker
Repo: psimaker/vaultsync PR: 38
File: ios/VaultSync/Views/ContentView.swift:605-611
Timestamp: 2026-06-10T18:47:10.724Z
Learning: In the SwiftUI codebase under ios/VaultSync, do not flag missing localization for SwiftUI string literals used as Text("…") or DisclosureGroup("…") titles/labels. In SwiftUI, these string literals are treated as LocalizedStringKey and resolve via the app’s Localizable.strings automatically—so they only need attention if the corresponding key is actually missing. Only require an explicit localization helper (e.g., L10n.tr(…)) when the string is not being passed through SwiftUI’s LocalizedStringKey path (e.g., plain String values provided to non-SwiftUI APIs).
Applied to files:
ios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Views/SyncIssuesView.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/PathCollisionGuard.swiftios/VaultSync/Services/SyncthingManager.swift
🔇 Additional comments (25)
ios/VaultSync/en.lproj/Localizable.strings (1)
620-624: LGTM!ios/VaultSync/de.lproj/Localizable.strings (1)
620-624: LGTM!ios/VaultSync/es.lproj/Localizable.strings (1)
620-624: LGTM!ios/VaultSync/zh-Hans.lproj/Localizable.strings (1)
620-624: LGTM!CHANGELOG.md (1)
7-7: LGTM!Also applies to: 11-12
go/bridge/folders.go (1)
221-265: LGTM!go/bridge/folders_test.go (1)
240-315: LGTM!go/bridge/pendingfolders.go (1)
8-9: LGTM!Also applies to: 108-118, 177-183
go/bridge/pendingfolders_test.go (1)
78-117: LGTM!ios/VaultSync/Services/SyncBridgeService.swift (1)
130-138: LGTM!ios/VaultSync/Services/PathCollisionGuard.swift (4)
43-62: LGTM!
64-101: LGTM!
103-157: LGTM!
159-191: LGTM!ios/VaultSync/Services/VaultManager.swift (2)
232-282: LGTM!
187-202: LGTM!ios/VaultSyncTests/VaultManagerSharePathTests.swift (1)
1-107: LGTM!ios/VaultSyncTests/PathCollisionGuardTests.swift (1)
1-205: LGTM!ios/VaultSync/Services/SyncthingManager.swift (4)
276-276: LGTM!
430-454: LGTM!
637-641: LGTM!
712-715: LGTM!ios/VaultSync/Views/SyncIssuesView.swift (3)
49-49: LGTM!
75-80: LGTM!
157-161: LGTM!
…45) CodeRabbit flagged user-derived values logged without privacy markers in VaultManager. Wrap folderName, path, and url.path with privacy: .private so debug logs and crash aggregators can't surface vault names or filesystem paths. folder.id, booleans, and counts stay visible for diagnostics.
1 participant
Fixes #45.
When a server shared more than one vault, VaultSync could assign the second vault the
same local folder as the first. Syncthing then merged both vaults into one directory
and pushed the mixed result back to every peer — silent data loss. This PR fixes the
cause and adds a migration shield for devices that already collided.
Prevention (new collisions can't happen)
VaultManager.resolveSharePath(pure, unit-tested): the Obsidian root staysa pure container; each vault syncs into its own
root/<name>. The collapse-into-rootshortcut is honoured only while the root is free; a same-named second vault gets a
deterministic
<name> (2)suffix instead of being merged.AcceptPendingFolderhard floor: rejects a local path already used by anotherfolder (
filepath.Clean+ case-insensitive compare for case-folding APFS), so twofolder IDs can never point at one directory — even if the client computed a colliding
path.
Migration shield (devices an older version already merged)
The guard above only prevents new collisions; a device that already merged two vaults
under 1.6.0/1.7.0 keeps mixing them on every sync.
accept-time guard uses; a path held by ≥2 folders is an active collision
(
PathCollisionGuard, pure + unit-tested).SetFolderPaused,recorded in a serialized
UserDefaultssidecar). It never auto-resumes, so a deliberateresume for recovery is respected; a failed pause is retried on the next launch.
list with a localized explanation and recovery steps. No automated
delete/rename/re-accept — recovery stays manual and user-driven.
Recovery: remove the affected vault on iPhone and the guard re-adds it into its own
folder (unpaused); if files are already mixed, restore the clean copy on the desktop
first.
Tests
make -C go test(incl.TestAcceptPendingFolderPathCollision,TestSetFolderPaused).VaultManagerSharePathTests(9),
FolderPathReconcilerTests(11),PathCollisionGuardTests(13).Ships in 1.7.1.
Summary
This PR fixes issue
#45, where multiple vaults from the same Syncthing server were automatically assigned the same local folder on iOS, causing silent data loss as both vaults merged into one directory and synced the mixed content back to peers.Solution Architecture
The fix implements two defensive layers:
Prevention (new collisions):
VaultManager.resolveSharePathensures each vault syncs into its ownroot/<name>subdirectory rather than collapsing into the root. Name collisions receive deterministic numeric suffixes (<name> (2),(3), etc.) instead of merging.AcceptPendingFolderhard-rejects any local path already used by another folder, using path normalization with case-insensitive comparison (critical for APFS case folding on iOS) to prevent duplicate path assignments at the sync engine level.Migration shield (already-merged vaults):
PathCollisionGuarddetects path collisions on launch by grouping folders by canonicalized, lowercased paths.UserDefaults) and never auto-resume, preserving user control.User-Visible Changes
Test Coverage
TestSetFolderPaused(pause/resume idempotency, Syncthing lifecycle) andTestAcceptPendingFolderPathCollision(path collision rejection with case-insensitive matching).VaultManagerSharePathTests(33 tests covering path resolution across single/multi-vault scenarios, naming collisions, case-insensitive handling)PathCollisionGuardTests(collision detection, group logic, auto-pause once semantics, failure retry behavior)FolderPathReconcilerTests(integration with collision guard during path reconciliation)Changelog
Updated for version 1.7.1 (2026-06-15) with fixes for both new collision prevention and detection/pausing of existing collisions.