fix: rebase folder paths on launch + guided vault removal (#25) - #32
Conversation
iOS does not keep an app's data-container path stable across reinstall, restore, or migration, but Syncthing persists absolute folder paths. A stale path left a vault stuck on "VaultSync cannot access this folder" with no way to recover (issue #25). Go bridge: - SetFolderPath rewrites a folder's path in place, preserving its devices, label, .stignore, and index DB (the DB is keyed by folder ID, so the folder restarts at the new path without re-hashing or re-downloading). Refuses a non-existent target so a folder is never pointed at an empty directory. - EnsureDefaultIgnores moves the default-ignore read-merge-write into Go and aborts on a read error instead of overwriting a populated .stignore with just the defaults. iOS: - FolderPathReconciler re-derives every folder's path from the bookmark-resolved Obsidian root on each engine start (foreground and the two background start paths), silently rebasing a moved container. A per-folder relative-path sidecar is recorded at accept time and backfilled for already-healthy folders on first launch. - Folders that genuinely can't be reached surface a calm "Needs Attention" card and a per-vault "Remove Vault" action (with "Reconnect to Obsidian" where applicable) instead of an inert error. - New strings localized in English, German, Spanish, and Simplified Chinese. Tests: new Go bridge tests (SetFolderPath, EnsureDefaultIgnores incl. the read-error/no-overwrite guard) and 11 FolderPathReconciler tests; full iOS suite (68 tests) and the Go bridge suite are green.
Defense in depth from the #25 architecture review. A rebase now verifies the target directory holds THIS folder's Syncthing marker (.stfolder/syncthing-folder-<hash(folderID)>.txt) instead of only checking that a directory exists. The fingerprint file proves the target was this very folder's root, so a folder can never be pointed at an empty or foreign directory — which a send-receive folder would otherwise treat as "all files deleted" and propagate to peers. Also makes the CHANGELOG narrative honest: a change to the app's own storage heals automatically, but a device restore/migration needs a one-time Obsidian-folder reconnect (index-preserving, no re-download). Go bridge tests extended (empty dir and foreign-marker dir refused; valid rebase requires the marker). Full Go bridge suite and the iOS suite (68 tests) are green.
|
Warning Review limit reached
More reviews will be available in 23 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more 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 (7)
📝 WalkthroughWalkthroughThis PR enables automatic folder-path recovery when iOS sandbox containers relocate (after reinstall, backup restore, or migration). A Go bridge layer ( ChangesFolder Path Resilience Recovery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 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 |
psimaker
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/VaultSync/Services/SyncthingManager.swift (1)
342-358:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFiltering the issue card is not enough if the remediation still rescans unreachable folders.
This hides unreachable folders from the generic issue count, but the Sync Issues action still calls
rescanFailedVaults(), which usesfolderIDsWithErrorsand therefore includes the same unreachable IDs. When reachable and unreachable errors coexist, the “rescan failed vaults” path still sends users through the no-op recovery this change is trying to remove.As per coding guidelines,
**/*: VaultSync syncs private Obsidian notes through Syncthing. Treat data loss, privacy leaks, security regressions, and broken sync behavior as high priority.🤖 Prompt for 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. In `@ios/VaultSync/Services/SyncthingManager.swift` around lines 342 - 358, The issue: unreachable folder IDs are filtered out of the SyncIssueItem list but the remediation still calls rescanFailedVaults() which uses folderIDsWithErrors and will include unreachable IDs; update the remediation path so rescanFailedVaults() is invoked only with the filtered erroredFolderIDs (or ensure rescanFailedVaults ignores unreachableFolders). Concretely, when creating the SyncIssueItem for .folderErrors (the SyncIssueItem construction that references folderIDsWithErrors and folderID: erroredFolderIDs.first), pass erroredFolderIDs into the rescan action handler (or alter rescanFailedVaults() to accept an explicit [FolderID] parameter and skip unreachableIDs) so the rescan excludes unreachableFolders and the remediation becomes a no-op only for truly unreachable entries.
🤖 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/FolderPathReconciler.swift`:
- Around line 129-135: The log statements in FolderPathReconciler (around the
loop calling env.setPath) currently mark folder.id as .public; remove any
exposure of the folder identifier from logs by omitting folder.id from the
interpolated messages or marking it private (e.g., do not use ", privacy:
.public"); update the logger.warning and logger.info calls in the block that
handles "Skip rebase", "Rebase failed", and the successful rebase to either
redact the identifier or log a generic message referencing only non-sensitive
context (keep references to env.setPath, folder.id, and the surrounding rebase
logic to locate the spots to change).
- Around line 34-56: The UserDefaults read-modify-write in loadRel/saveRel and
the mutating APIs setRel, removeRel, and reconcile are not synchronized and can
race between foreground/background threads; wrap all access to the rel sidecar
behind a single-serialized execution context (e.g., a dedicated actor or a
serial DispatchQueue) or expose an atomic update API that performs
load-modify-save as one synchronous step. Concretely, create a
FolderPathReconcilerStorage actor or serial queue responsible for loadRel(),
saveRel(_:), setRel(_:forFolder:), removeRel(forFolder:), and reconcile(...) and
move the UserDefaults reads/writes into that serialized context so callers
always await/dispatch into it, ensuring no concurrent access to relStoreKey or
the dictionary map.
In `@ios/VaultSync/Services/SyncthingManager.swift`:
- Around line 497-506: reconcileFolderPaths is currently using Task { } so
waitForFoldersForSyncRequest and refreshFolders run on the `@MainActor`; change
the task to run off the main actor (use Task.detached) so polling/bridge work
(waitForFoldersForSyncRequest and the FolderPathReconciler call) happens off the
UI thread, and only invoke refreshFolders back on the main actor via
MainActor.run when UI/actor-safety is required; locate reconcileFolderPaths and
update the Task usage, keeping FolderPathReconciler.reconcileLive and
waitForFoldersForSyncRequest logic intact and wrapping any refreshFolders call
in MainActor.run.
- Around line 718-724: applyDefaultIgnoresIfNeeded is running
SyncBridgeService.ensureDefaultIgnores on the MainActor which can block the UI;
change call sites so the bridge work runs off-main (invoke
SyncBridgeService.ensureDefaultIgnores from Task.detached or an async
non-@MainActor context), make ensureDefaultIgnores/BridgeEnsureDefaultIgnores
cancellation-aware (check Task.isCancelled or use withTaskCancellationHandler)
and only hop back to `@MainActor` for any state updates or logging that must run
on the UI actor (e.g., update UI state or call logger.warning on the main
actor).
---
Outside diff comments:
In `@ios/VaultSync/Services/SyncthingManager.swift`:
- Around line 342-358: The issue: unreachable folder IDs are filtered out of the
SyncIssueItem list but the remediation still calls rescanFailedVaults() which
uses folderIDsWithErrors and will include unreachable IDs; update the
remediation path so rescanFailedVaults() is invoked only with the filtered
erroredFolderIDs (or ensure rescanFailedVaults ignores unreachableFolders).
Concretely, when creating the SyncIssueItem for .folderErrors (the SyncIssueItem
construction that references folderIDsWithErrors and folderID:
erroredFolderIDs.first), pass erroredFolderIDs into the rescan action handler
(or alter rescanFailedVaults() to accept an explicit [FolderID] parameter and
skip unreachableIDs) so the rescan excludes unreachableFolders and the
remediation becomes a no-op only for truly unreachable entries.
🪄 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: aad6c6b1-a22c-4f06-b9fe-aa52cfb107ea
📒 Files selected for processing (16)
CHANGELOG.mdgo/bridge/folders.gogo/bridge/folders_test.gogo/bridge/folderstatus.goios/VaultSync/App/VaultSyncApp.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/FolderPathReconciler.swiftios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Views/ContentView.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/es.lproj/Localizable.stringsios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSyncTests/FolderPathReconcilerTests.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: Go Tests
🧰 Additional context used
📓 Path-based instructions (7)
**/*.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/App/VaultSyncApp.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/FolderPathReconciler.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSyncTests/FolderPathReconcilerTests.swiftios/VaultSync/Views/ContentView.swift
ios/**/*.swift
📄 CodeRabbit inference engine (README.md)
ios/**/*.swift: Follow Swift API Design Guidelines for Swift code
Use Swift strict concurrency where applicable
Use SwiftUI for iOS user interface development
Use BGAppRefreshTask and BGContinuedProcessingTask for background execution on iOS 18+
Implement APNs silent notifications via Cloud Relay for push wake-ups
Files:
ios/VaultSync/App/VaultSyncApp.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/FolderPathReconciler.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSyncTests/FolderPathReconcilerTests.swiftios/VaultSync/Views/ContentView.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/App/VaultSyncApp.swiftios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/FolderPathReconciler.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSyncTests/FolderPathReconcilerTests.swiftios/VaultSync/Views/ContentView.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/App/VaultSyncApp.swiftios/VaultSync/es.lproj/Localizable.stringsios/VaultSync/de.lproj/Localizable.stringsCHANGELOG.mdgo/bridge/folderstatus.goios/VaultSync/Services/VaultManager.swiftios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSync/Services/FolderPathReconciler.swiftios/VaultSync/Services/SyncthingManager.swiftgo/bridge/folders.goios/VaultSyncTests/FolderPathReconcilerTests.swiftios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/Views/ContentView.swiftgo/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
{go,notify}/**/*.go
📄 CodeRabbit inference engine (README.md)
Follow standard Go conventions for Go code
Files:
go/bridge/folderstatus.gogo/bridge/folders.gogo/bridge/folders_test.go
go/**/*.go
📄 CodeRabbit inference engine (README.md)
Embed Syncthing 2.x engine via Go/gomobile as .xcframework in iOS builds
Files:
go/bridge/folderstatus.gogo/bridge/folders.gogo/bridge/folders_test.go
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/folderstatus.gogo/bridge/folders.gogo/bridge/folders_test.go
🔇 Additional comments (5)
ios/VaultSync/de.lproj/Localizable.strings (1)
576-585: LGTM!ios/VaultSync/en.lproj/Localizable.strings (1)
576-585: LGTM!ios/VaultSync/es.lproj/Localizable.strings (1)
576-585: LGTM!ios/VaultSync/zh-Hans.lproj/Localizable.strings (1)
576-585: LGTM!go/bridge/folders.go (1)
194-218: ⚡ Quick winFix review: marker fingerprint check matches Syncthing’s default
.stfoldercontract — Syncthing’s default.stfolderincludes a deterministicsyncthing-folder-<hash>.txtfingerprint derived fromsha256(folderID)truncated to the first 3 bytes, and for customMarker Nameit only requires the marker entry to exist (it isn’t created automatically). The currentverifyFolderMarkerlogic (including theh[:3]filename) and the unit tests’writeFolderMarkerhelper align with that behavior.
- FolderPathReconciler: serialize the rel sidecar behind a serial queue and have reconcile write per-folder atomically (recordRel) from a read snapshot instead of a bulk overwrite, so concurrent foreground and background reconciles can't drop a mapping. Stop logging folder IDs as public. - SyncthingManager: run reconcileFolderPaths' folder-wait + reconcile off the main actor (Task.detached), hopping back only for refreshFolders; make applyDefaultIgnoresIfNeeded nonisolated and dispatch its three call sites off-main so the .stignore read-merge-write never blocks the UI. - ContentView: "rescan failed vaults" now skips unreachable folders (a rescan cannot fix a stale path). Full iOS suite (68 tests) green.
Mark the #25 folder-path resilience and Sync Filters hardening as the 1.5.1 release in the changelog, lead the README "What's New" with the 1.5.1 fix, and bump CFBundleShortVersionString/CFBundleVersion in project.yml for the app and widget targets.
1 participant
Why
iOS does not keep an app's data-container path stable across reinstall, restore, or device migration, but Syncthing
persists absolute folder paths in its config. A stale path left a vault permanently stuck on "VaultSync cannot access
this folder" with no in-app way to recover (#25).
What
FolderPathReconciler): on every engine start (foreground + both background startpaths) each folder's path is re-derived from the bookmark-resolved Obsidian root + a per-folder relative-path sidecar.
A change to the app's own storage is repaired automatically; the per-folder mapping is recorded at accept time and
backfilled for healthy folders on first launch.
SetFolderPath(Go bridge): rewrites a folder's path in place, preserving devices, label,.stignore, and theindex DB — a Path change is restart-only in
lib/modeland never drops the DB, so there is no re-hash and nore-download. It refuses to rebase unless the target holds this folder's own marker fingerprint
(
.stfolder/syncthing-folder-<hash(folderID)>.txt), so a folder can never be pointed at an empty or foreign directory(which a send-receive folder would treat as "all files deleted" and propagate to peers).
Vault action (plus Reconnect to Obsidian where the folder maps to it) instead of an inert error.
EnsureDefaultIgnores(Go bridge): moves the default-ignore read-merge-write into Go and aborts on a read errorinstead of overwriting a populated
.stignorewith just the defaults.Honest scope
A change to the app's own storage heals with zero user action. After a device restore or migration the Obsidian
security-scoped bookmark often has to be re-granted once; that single reconnect then rebases every vault onto its
existing data (index-preserving). This is a platform limit — the cross-app bookmark is a non-programmatic single point
of failure — and it still goes further than other iOS Syncthing clients, which require a full re-sync.
Testing
SetFolderPath(in-place rebase preserves devices; empty-dir, foreign-marker, and non-existenttargets refused; no-op when unchanged) and
EnsureDefaultIgnores(adds only missing patterns, preserves customorder, idempotent, aborts on read error). Full Go bridge suite green.
FolderPathReconcilerunit tests (rebase / backfill / no-op / unmappable).Fixes #25.
Overview
This PR fixes issue
#25by making Syncthing folder paths resilient to iOS container path changes that occur during app reinstall, restore, or device migration. Previously, when iOS relocated an app's sandboxed storage, vaults would become stuck with a "VaultSync cannot access this folder" error with no in-app recovery path.User-Visible Changes
Path Reconciliation & Recovery
Sync Filter Handling
.stignorepatterns are now read, merged, and written atomically; read errors prevent accidental overwrites of existing filtersLocalization
Sync Behavior Impact
.stfolder/syncthing-folder-<hash(folderID)>.txt) to prevent rebasing onto wrong/empty directoriesBackground Execution
Test Coverage
FolderPathReconcilercovering rebase, backfill, and sidecar persistence scenariosSetFolderPath(marker verification, device preservation, edge cases) andEnsureDefaultIgnores(merge logic, permission errors)Technical Implementation