Skip to content

v1.3.2: Skip-Family fix (#8) + Reconnecting UX polish - #9

Merged
psimaker merged 23 commits into
mainfrom
fix/issue-8-conflict-skip-family
May 23, 2026
Merged

v1.3.2: Skip-Family fix (#8) + Reconnecting UX polish#9
psimaker merged 23 commits into
mainfrom
fix/issue-8-conflict-skip-family

Conversation

@psimaker

@psimaker psimaker commented May 23, 2026

Copy link
Copy Markdown
Owner

Summary

Two independent fixes shipping together as v1.3.2 (build 24):

1. Skip-Family — closes #8 (reported by @vitaly74)

Tapping "Always skip on this iPhone" in the conflict resolver previously added only the original file's path to .stignore, so a fresh sync-conflict-… copy with a new timestamp would arrive from the desktop and the conflict reappeared. The Skip flow now:

  • Writes a pair of patterns to .stignore: the original path AND a <path>.sync-conflict-* glob
  • Actively deletes any existing conflict copies of that file from disk
  • Rescans the folder and refreshes the conflict cache so the conflict disappears immediately
  • Renders the pair as one row in Sync Filters → Custom Patterns with a "+ conflict copies" caption; swipe-to-delete removes both lines atomically

2. Reconnecting Grace Period — internal UX polish

When VaultSync resumes after the app has been away from the foreground for a while, the home screen no longer briefly flashes a "1 Required Device Is Disconnected" warning while the embedded Syncthing process is still rebuilding its connection. Instead, the sync status reads "Reconnecting…" with a calm system spinner for up to 30 seconds, then either returns to "All Synced" or surfaces the existing warning if the peer is genuinely offline.

Implemented via per-device disconnect-timestamp tracking on SyncthingManager, with a single applyDeviceList(_:) entry point that keeps the dictionary consistent with the devices array.

Changes

  • Go bridge: RemoveConflictFilesForOriginal(folderID, originalPath) -> JSON for active conflict-copy cleanup
  • Swift: SyncthingManager.conflictGlob(forOriginalPath:), skipFileAndCleanupConflicts(folderID:originalPath:), new SkipFamilyEntry/SkipFamilyGrouping types, disconnectedSince state + reconnectingRequiredDeviceIDs property
  • UI: ConflictDiffView wired to new Skip flow, IgnorePatternsView renders paired entries as one row, ContentView dashboard shows "Reconnecting…" with ProgressView spinner
  • Localization: 7 new strings across en/de/zh-Hans (4 for Skip-Family + 3 for Reconnecting, with singular/plural split for counts)
  • Versioning: project.yml bumped to 1.3.2 / build 24, CHANGELOG updated, README "What's New" refreshed, UX-spec §6 rewritten

Test plan

Automated

  • Go bridge: `go test -tags noassets ./bridge/` — passes (incl. dotted-stem regression, path-traversal, root-path rejection)
  • iOS: `xcodebuild test` — 56 tests pass (13 SkipFamilyTests + 7 ReconnectingGracePeriodTests + 36 pre-existing)

Manual smoke (verified on device)

  • Skip-Family — divergent edits on iPhone+Desktop → conflict appears → Skip → alert mentions "and its conflict copies" + removed count → diff view dismisses → conflict gone from Sync Issues → Sync Filters shows one grouped row → next divergence does NOT bring conflict back → swipe-delete removes both `.stignore` lines
  • Reconnecting — background 5+ min + foreground → "Reconnecting…" with spinner, no warning, then "All Synced" → Wi-Fi off >30s → real warning surfaces normally → en/de/zh-Hans strings render cleanly

v1.3.2: Skip-Family fix + Reconnecting grace period

This release addresses two issues affecting the sync experience:

Skip-Family fix (closes #8)

Resolves a bug where choosing "Always skip on this iPhone" only prevented the original file from syncing but allowed new conflict copies to reappear. The fix now:

  • Writes both the original file path and a <path>.sync-conflict-* glob pattern to .stignore
  • Actively deletes any existing sync-conflict copies from the local filesystem
  • Rescans the folder and refreshes the conflict cache immediately
  • Presents both patterns as a single grouped row in Sync Filters with a "+ conflict copies" label (atomic deletion when swiped)

This ensures that skipping a file truly eliminates it and all future conflict variants from syncing.

Reconnecting grace period (UX polish)

Prevents a transient "Required Device Is Disconnected" warning when the app returns to foreground while Syncthing rebuilds connections. The dashboard now displays "Reconnecting…" with a spinner for up to 30 seconds; after that period, it either returns to normal status or surfaces any genuine disconnection. Implemented via per-device disconnect-timestamp tracking in SyncthingManager.

Changes

  • Go bridge: Added RemoveConflictFilesForOriginal(folderID, originalPath) to delete matching *.sync-conflict-* files
  • iOS: New helpers (conflictGlob, skipFileAndCleanupConflicts), SkipFamilyEntry/SkipFamilyGrouping types for pattern pairing, per-device disconnect tracking, and reconnecting status in dashboard
  • UI: Grouped conflict patterns in Sync Filters, "Reconnecting…" state in ContentView with spinner, updated confirmation alerts
  • Localization: 7 new strings for reconnecting/conflict messages (en/de/zh-Hans)
  • Tests: New unit tests for RemoveConflictFilesForOriginal, conflictGlob, SkipFamilyGrouping.group(), and reconnect grace-period behavior
  • Version bump: 1.3.1 (build 23) → 1.3.2 (build 24)

Review Change Stack

psimaker added 18 commits May 23, 2026 08:25
Spec for the fix to issue #8: 'Always skip on this iPhone' currently
only ignores the original path, so future sync-conflict copies bypass
the filter and conflicts reappear. Design adds a paired ignore pattern
(original + sync-conflict-* glob), actively removes existing conflict
copies, and groups the pair as a single row in the Sync Filters list.
Removes every sync-conflict copy of a given file in a folder, leaving
the original on disk. Used by the 'Always skip on this iPhone' flow to
actively clear conflict-copy leftovers when the user opts out of a file.
Pure helpers shared between the conflict-resolver Skip flow and the
Sync Filters list rendering. Tests cover root files, nested paths,
extension-less filenames, mixed-extension stems, and pair detection.
Single-call API for the conflict-resolver Skip flow. Writes the pair of
ignore patterns, deletes existing conflict copies on disk, rescans, and
refreshes the conflict cache. Returns the number of removed conflict
copies so the UI can surface it in the success alert.
Replace the single-pattern addIgnorePattern call with the full Skip
flow. Confirmation alert now mentions conflict copies and, when any
existed on disk, the number that were removed. Dismisses the diff view
on OK since the conflict no longer exists.
A paired '<X>' + '<X>.sync-conflict-*' renders as a single Custom
Patterns entry with a '+ conflict copies' caption. Swipe-to-delete
removes both lines from .stignore atomically. Orphan singletons keep
their existing single-line rendering.
Adds the new 'conflict copies' alert variant plus singular/plural
variants of the removed-count sentence and the '+ conflict copies'
caption used by the Sync Filters list, for en / de / zh-Hans.
Add a per-device 'first observed disconnected' timestamp. Within a 30s
window the device counts as 'reconnecting' (calm dashboard state, no
warning); after the window it surfaces as a real disconnect via the
existing Sync-Issues path. Single entry point applyDeviceList(_:) keeps
the timestamp dictionary consistent with the devices array.
Insert a new branch in syncStatusText / syncStatusIcon / syncStatusColor
that triggers when reconnectingRequiredDeviceIDs is non-empty. Dashboard
icon swaps to a system ProgressView spinner for the iOS-native look
familiar from pull-to-refresh, and a 'Restoring connection to N
device(s)' caption appears under the status text.
Adds the three new keys consumed by the dashboard reconnecting branch.
@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@psimaker, we couldn't start this review because you've used your available PR reviews for now.

Your plan currently allows 1 review/hour. Refill in 19 minutes and 45 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e4db6269-39c5-4bfc-9528-fc6561323846

📥 Commits

Reviewing files that changed from the base of the PR and between 3c64d42 and 3dc1691.

📒 Files selected for processing (6)
  • docs/sync-filters-ux.md
  • ios/VaultSync/Models/SkipFamily.swift
  • ios/VaultSync/Services/SyncthingManager.swift
  • ios/VaultSync/Views/ConflictDiffView.swift
  • ios/VaultSync/Views/ContentView.swift
  • ios/VaultSyncTests/SkipFamilyTests.swift
📝 Walkthrough

Walkthrough

This PR implements v1.3.2's two headline features: a "Skip Family" conflict resolver that atomically ignores a file and its Syncthing-generated conflict copies (deleting existing variants immediately), and a grace-period model for required-device disconnects (showing "Reconnecting…" for 30 seconds before surfacing the offline alert). Cross-platform infrastructure includes Go file cleanup, Swift model grouping, and updated UX throughout the sync dashboard and conflict/filter views.

Changes

Skip Family Conflict Resolution & Device Reconnect Grace Period

Layer / File(s) Summary
Skip Family model and grouping logic
ios/VaultSync/Models/SkipFamily.swift, ios/VaultSyncTests/SkipFamilyTests.swift
Defines SkipFamilyEntry to represent singleton or paired original-plus-conflict-glob patterns, with underlyingLines generating the underlying .stignore entries. SkipFamilyGrouping.group(customLines:) pairs raw ignore lines by stem/extension matching, preventing orphan conflict globs via a pre-pass.
Conflict file removal infrastructure
go/bridge/conflicts.go, go/bridge/conflicts_test.go, ios/VaultSync/Services/SyncBridgeService.swift
Go RemoveConflictFilesForOriginal() scans and safely deletes all *.sync-conflict-* variants of a given file within a folder, with path validation and JSON error reporting. Swift wrapper removeConflictFilesForOriginal() decodes the bridge response and returns removal counts or error messages. Tests cover nested paths, dotted names, idempotency, and path-traversal rejection.
Device disconnect grace-period tracking
ios/VaultSync/Services/SyncthingManager.swift, ios/VaultSyncTests/ReconnectingGracePeriodTests.swift
SyncthingManager tracks per-device disconnect start times via disconnectedSince, introduces a 30-second reconnectGracePeriod, and adds applyDeviceList() to maintain timestamps across state changes. New reconnectingRequiredDeviceIDs list returns devices within the grace window; disconnectedRequiredDeviceIDs rewritten to classify devices beyond grace plus never-seen required devices. Includes seven comprehensive grace-window state-transition test cases.
Skip file and cleanup orchestration
ios/VaultSync/Services/SyncthingManager.swift
skipFileAndCleanupConflicts(folderID:originalPath:) atomically updates .stignore with paired patterns, removes conflict-copy files via bridge, rescans the folder, and refreshes the conflict cache for immediate UI update. Static conflictGlob(forOriginalPath:) generates safe conflict globs with defensive handling of root-like inputs. DEBUG test hooks added for state injection.
ConflictDiffView skip flow integration
ios/VaultSync/Views/ConflictDiffView.swift
skipThisFile() now calls skipFileAndCleanupConflicts(), capturing the removed-conflict count and displaying it in the confirmation alert with singular/plural variants. Tracks removed count via new skipRemovedCount state.
Reconnecting status UI and sync dashboard
ios/VaultSync/Views/ContentView.swift
Dashboard now branches on isReconnecting state, showing a teal progress indicator and "Restoring connection…" message with device count. syncStatusIcon, syncStatusColor, and syncStatusText extended to integrate reconnecting state while preserving error/running precedence.
Sync Filters view grouped entry rendering and deletion
ios/VaultSync/Views/IgnorePatternsView.swift
Custom patterns now render via SkipFamilyGrouping.group(), displaying grouped entries with "+ conflict copies" caption when applicable. Deletion refactored to remove all underlyingLines of each grouped entry, using deduplication to prevent duplicate line removal.
Localization strings for reconnection and skip messaging
ios/VaultSync/en.lproj/Localizable.strings, ios/VaultSync/de.lproj/Localizable.strings, ios/VaultSync/zh-Hans.lproj/Localizable.strings
New status strings for "Reconnecting…" and "Restoring connection to N devices" (singular/plural across all three languages). Skip/conflict-copy messaging explains that conflict copies will no longer sync, reports removal counts, and labels the "+ conflict copies" grouping in Sync Filters.
Release documentation and version updates
.gitignore, CHANGELOG.md, README.md, docs/sync-filters-ux.md, ios/project.yml
Version bumped to 1.3.2 (build 24) for app and widget. CHANGELOG and README describe the two fixed issues. UX spec updated with detailed "Skip Family" semantics: paired patterns, immediate cleanup, no deletion of original file, prevention of reintroduced conflicts through glob matching. Generated docs directory added to .gitignore.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes


Possibly related PRs

  • psimaker/vaultsync#2: Foundational "Sync Filters" conflict-skip UX that this PR builds on by adding "Skip Family" paired-pattern logic and on-disk conflict cleanup.

Poem

A sync conflict blooms—twin files in strife,
But "Skip Family" unites them in ignore-life.
One glob, two patterns, cleanup done right,
And "Reconnecting…" holds its grace through the night. 🔄✨

🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Bounded Ios Background Work ⚠️ Warning skipThisFile() executes heavy I/O and bridge calls (file deletion, rescans) synchronously on main thread, blocking UI without timeout or cancellation support. Move work to background Task, marshal UI updates back to MainActor with cancellation/timeout handling per review comment Lines 209-220.
Title check ❓ Inconclusive The PR title uses a version-first format rather than conventional-commit style (fix:, feat:, docs:, chore:, ci:); it reads as a release summary rather than a descriptive commit title. Consider reformatting as a conventional-commit style title (e.g., 'feat: add skip-family conflict handling and reconnecting grace period') while preserving the substance.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR fully addresses the linked issue #8 requirements: it implements a skip-family fix that writes paired .stignore patterns (original + conflict glob), actively deletes existing conflict copies, rescans immediately, and renders them as a grouped row in Sync Filters.
Out of Scope Changes check ✅ Passed The PR introduces a second feature (reconnecting grace period / UX polish) beyond the core #8 fix. While not explicitly required by #8, it is scoped and justified as UX polish and documented in objectives; all changes directly support the two announced features.
No Private Note Leakage ✅ Passed No logging, analytics, crash reporting, diagnostics, or network requests containing private note contents, filenames, vault paths, API keys, tokens, or other sensitive data were introduced.
Bridge Contract Compatibility ✅ Passed Go RemoveConflictFilesForOriginal and Swift wrapper maintain bridge compatibility: gomobile-compatible types, matching JSON structure, proper empty-string success convention, and comprehensive tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-8-conflict-skip-family

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
docs/sync-filters-ux.md (1)

121-121: 💤 Low value

Optional: Add language specifier to fenced code block.

The fenced code block at line 121 is missing a language identifier. Adding ```text or ```plaintext would resolve the markdownlint warning without changing the content.

📝 Proposed fix
-```
+```text
 ⋯ menu
 └─ Always skip on this iPhone
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @docs/sync-filters-ux.md at line 121, The fenced code block that contains the
snippet starting with "⋯ menu" is missing a language identifier; update the
opening fence for that block (the fenced code block containing "⋯ menu" /
"Always skip on this iPhone") to use a plaintext language specifier such as
text (or plaintext) so the markdownlint warning is resolved while keeping
the content unchanged.


</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

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/Models/SkipFamily.swift:

  • Around line 35-55: The current pairing logic in the loop that builds
    pairedGlobs and the second loop that emits SkipFamilyEntry uses
    SyncthingManager.conflictGlob(forOriginalPath:) for every line, which can
    wrongly pair wildcard or directory rules; update both loops to only consider
    pairing when the original path is a concrete file path (e.g., exclude patterns
    containing "*" or "?" or trailing "/" or any directory/glob-specific tokens).
    Concretely: before calling conflictGlob(forOriginalPath:) or adding to
    pairedGlobs/creating a paired SkipFamilyEntry, add a guard that verifies the
    candidate line (and the derived glob) is a concrete file path (use the same
    glob-detection logic in both the first loop that populates pairedGlobs and the
    second loop that appends to result); keep using the existing symbols
    (customLines, pairedGlobs, lineSet, conflictGlob(forOriginalPath:), consumed,
    SkipFamilyEntry, hasConflictGlob) when implementing the guard.

In @ios/VaultSync/Services/SyncthingManager.swift:

  • Around line 1743-1751: The current code ignores errors from
    SyncBridgeService.removeConflictFilesForOriginal and
    SyncBridgeService.rescanFolder and always returns success; change the logic so
    you capture and propagate failures from both
    SyncBridgeService.removeConflictFilesForOriginal and
    SyncBridgeService.rescanFolder (e.g., check for a thrown error or a Result
    value), only call refreshConflicts and return the removed list when both calls
    succeed, and return the encountered error (or a mapped error) otherwise so
    callers see cleanup/rescan failures instead of silent success.

In @ios/VaultSync/Views/ConflictDiffView.swift:

  • Around line 209-220: skipThisFile currently calls
    SyncthingManager.skipFileAndCleanupConflicts(...) synchronously on the UI
    thread; move the heavy work into a background Task (e.g., Task.detached or Task
    { } with Task.checkCancellation points) that calls
    SyncthingManager.skipFileAndCleanupConflicts(folderID:originalPath:), handles
    cancellation, then marshals only UI updates back to the MainActor to set
    skipErrorMessage, showSkipError, skipRemovedCount and showSkipConfirmation;
    ensure any filesystem helpers like removeConflictFilesForOriginal(...) and
    refreshConflicts() run off the main thread and that errors from
    skipFileAndCleanupConflicts are captured and delivered to the MainActor for
    presentation.

In @ios/VaultSync/Views/ContentView.swift:

  • Around line 137-160: The reconnecting spinner and subtitle are shown whenever
    isReconnecting is true, which can clash with higher-priority states reflected by
    syncStatusText; update the UI to only show reconnecting visuals when both
    isReconnecting is true and the current status precedence allows it (i.e., the
    same guard used to decide/derive syncStatusText). Concretely, wrap the
    spinner/Image selection and the VStack subtitle branch (currently using
    isReconnecting and syncthingManager.reconnectingRequiredDeviceIDs) behind the
    same precedence check/flag you use to compute syncStatusText (or a derived
    boolean like showReconnectingUI) so that error/folder-issue states take priority
    and suppress reconnecting visuals.

Nitpick comments:
In @docs/sync-filters-ux.md:

  • Line 121: The fenced code block that contains the snippet starting with "⋯
    menu" is missing a language identifier; update the opening fence for that block
    (the fenced code block containing "⋯ menu" / "Always skip on this iPhone") to
    use a plaintext language specifier such as text (or plaintext) so the
    markdownlint warning is resolved while keeping the content unchanged.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Repository UI

**Review profile**: CHILL

**Plan**: Pro Plus

**Run ID**: `5ecbcaa0-5bd1-4a7a-9d4d-38ec5dbff41f`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 4af820f36bc3d6c6c278a36cfa35d420fa575f89 and 3c64d426fa5fc061c6795856d2e1dcb033862a12.

</details>

<details>
<summary>📒 Files selected for processing (18)</summary>

* `.gitignore`
* `CHANGELOG.md`
* `README.md`
* `docs/sync-filters-ux.md`
* `go/bridge/conflicts.go`
* `go/bridge/conflicts_test.go`
* `ios/VaultSync/Models/SkipFamily.swift`
* `ios/VaultSync/Services/SyncBridgeService.swift`
* `ios/VaultSync/Services/SyncthingManager.swift`
* `ios/VaultSync/Views/ConflictDiffView.swift`
* `ios/VaultSync/Views/ContentView.swift`
* `ios/VaultSync/Views/IgnorePatternsView.swift`
* `ios/VaultSync/de.lproj/Localizable.strings`
* `ios/VaultSync/en.lproj/Localizable.strings`
* `ios/VaultSync/zh-Hans.lproj/Localizable.strings`
* `ios/VaultSyncTests/ReconnectingGracePeriodTests.swift`
* `ios/VaultSyncTests/SkipFamilyTests.swift`
* `ios/project.yml`

</details>

</details>

<details>
<summary>📜 Review details</summary>

<details>
<summary>⏰ 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)</summary>

* GitHub Check: Bridge Schema Regression

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>📓 Path-based instructions (7)</summary>

<details>
<summary>**/*.swift</summary>


**📄 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.swift`
- `ios/VaultSyncTests/ReconnectingGracePeriodTests.swift`
- `ios/VaultSyncTests/SkipFamilyTests.swift`
- `ios/VaultSync/Models/SkipFamily.swift`
- `ios/VaultSync/Views/ContentView.swift`
- `ios/VaultSync/Views/ConflictDiffView.swift`
- `ios/VaultSync/Views/IgnorePatternsView.swift`
- `ios/VaultSync/Services/SyncthingManager.swift`

</details>
<details>
<summary>ios/**/*.swift</summary>


**📄 CodeRabbit inference engine (README.md)**

> Follow Swift API Design Guidelines
> 
> Use Swift strict concurrency where applicable
> 
> iOS background execution should use BGAppRefreshTask and BGContinuedProcessingTask where available
> 
> Support VoiceOver and Dynamic Type accessibility throughout the app

Files:
- `ios/VaultSync/Services/SyncBridgeService.swift`
- `ios/VaultSyncTests/ReconnectingGracePeriodTests.swift`
- `ios/VaultSyncTests/SkipFamilyTests.swift`
- `ios/VaultSync/Models/SkipFamily.swift`
- `ios/VaultSync/Views/ContentView.swift`
- `ios/VaultSync/Views/ConflictDiffView.swift`
- `ios/VaultSync/Views/IgnorePatternsView.swift`
- `ios/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.swift`
- `ios/VaultSyncTests/ReconnectingGracePeriodTests.swift`
- `ios/VaultSyncTests/SkipFamilyTests.swift`
- `ios/VaultSync/Models/SkipFamily.swift`
- `ios/VaultSync/Views/ContentView.swift`
- `ios/VaultSync/Views/ConflictDiffView.swift`
- `ios/VaultSync/Views/IgnorePatternsView.swift`
- `ios/VaultSync/Services/SyncthingManager.swift`

</details>
<details>
<summary>**/*</summary>


**⚙️ 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.swift`
- `ios/project.yml`
- `README.md`
- `ios/VaultSyncTests/ReconnectingGracePeriodTests.swift`
- `ios/VaultSyncTests/SkipFamilyTests.swift`
- `ios/VaultSync/en.lproj/Localizable.strings`
- `docs/sync-filters-ux.md`
- `ios/VaultSync/Models/SkipFamily.swift`
- `go/bridge/conflicts.go`
- `ios/VaultSync/de.lproj/Localizable.strings`
- `ios/VaultSync/zh-Hans.lproj/Localizable.strings`
- `go/bridge/conflicts_test.go`
- `ios/VaultSync/Views/ContentView.swift`
- `ios/VaultSync/Views/ConflictDiffView.swift`
- `CHANGELOG.md`
- `ios/VaultSync/Views/IgnorePatternsView.swift`
- `ios/VaultSync/Services/SyncthingManager.swift`

</details>
<details>
<summary>ios/project.yml</summary>


**⚙️ CodeRabbit configuration file**

> `ios/project.yml`: This generates the Xcode project and Info.plist. Review changes for bundle ID,
> entitlements, background modes, URL schemes, signing settings, and accidental secret exposure.

Files:
- `ios/project.yml`

</details>
<details>
<summary>**/*.md</summary>


**⚙️ 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:
- `README.md`
- `docs/sync-filters-ux.md`
- `CHANGELOG.md`

</details>
<details>
<summary>go/**/*.go</summary>


**📄 CodeRabbit inference engine (README.md)**

> Follow Standard Go conventions

Files:
- `go/bridge/conflicts.go`
- `go/bridge/conflicts_test.go`

</details>
<details>
<summary>go/bridge/**/*.go</summary>


**⚙️ 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/conflicts.go`
- `go/bridge/conflicts_test.go`

</details>

</details><details>
<summary>🧠 Learnings (1)</summary>

<details>
<summary>📓 Common learnings</summary>

```
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-05-23T09:56:02.137Z
Learning: Use Conventional Commits for commit messages
```

```
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-05-23T09:56:02.137Z
Learning: Provide clear PR descriptions in pull requests
```

```
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-05-23T09:56:02.137Z
Learning: Syncs Obsidian vaults directly into Obsidian's iOS sandbox via Syncthing protocol
```

```
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-05-23T09:56:02.137Z
Learning: No note content, filenames, folder names, vault structure, or vault metadata should pass through Cloud Relay servers
```

```
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-05-23T09:56:02.137Z
Learning: Implement Markdown conflict resolution with side-by-side diffs for conflicting files
```

```
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-05-23T09:56:14.167Z
Learning: When a new folder is added, silently apply the Recommended preset set without showing the first-run sheet, to prevent workspace.json from generating immediate sync conflicts
```

```
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-05-23T09:56:14.167Z
Learning: For users updating from previous builds, keep existing `.Trash`, `.obsidian/workspace.json`, and `.obsidian/workspace-mobile.json` patterns on disk untouched and automatically show them as ON in derived state without migrating data
```

```
Learnt from: CR
Repo: psimaker/vaultsync

Timestamp: 2026-05-23T09:56:14.167Z
Learning: Use consistent terminology throughout the app: 'Sync Filters' for section titles, 'Skip on this iPhone' or 'Always skip on this iPhone' for CTAs, and 'Choose what gets synced to this iPhone' for explanatory copy, avoiding jargon like 'ignore patterns', 'exclusions', or 'filter rules'
```

</details>

</details><details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

<details>
<summary>docs/sync-filters-ux.md</summary>

[warning] 121-121: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

</details>

<details>
<summary>🔇 Additional comments (16)</summary><blockquote>

<details>
<summary>ios/VaultSyncTests/ReconnectingGracePeriodTests.swift (1)</summary><blockquote>

`4-143`: LGTM!

</blockquote></details>
<details>
<summary>ios/VaultSync/Views/ContentView.swift (1)</summary><blockquote>

`273-303`: LGTM!

</blockquote></details>
<details>
<summary>.gitignore (1)</summary><blockquote>

`48-48`: LGTM!

</blockquote></details>
<details>
<summary>CHANGELOG.md (1)</summary><blockquote>

`7-14`: LGTM!

</blockquote></details>
<details>
<summary>README.md (1)</summary><blockquote>

`102-105`: LGTM!

</blockquote></details>
<details>
<summary>docs/sync-filters-ux.md (1)</summary><blockquote>

`4-4`: LGTM!



Also applies to: 119-142

</blockquote></details>
<details>
<summary>ios/project.yml (1)</summary><blockquote>

`53-54`: LGTM!



Also applies to: 103-104

</blockquote></details>
<details>
<summary>ios/VaultSync/Views/ConflictDiffView.swift (1)</summary><blockquote>

`26-27`: LGTM!



Also applies to: 182-197

</blockquote></details>
<details>
<summary>ios/VaultSync/Views/IgnorePatternsView.swift (1)</summary><blockquote>

`80-89`: LGTM!



Also applies to: 112-118, 189-197

</blockquote></details>
<details>
<summary>ios/VaultSync/de.lproj/Localizable.strings (1)</summary><blockquote>

`330-332`: LGTM!



Also applies to: 482-485

</blockquote></details>
<details>
<summary>ios/VaultSync/en.lproj/Localizable.strings (1)</summary><blockquote>

`331-333`: LGTM!



Also applies to: 483-486

</blockquote></details>
<details>
<summary>ios/VaultSync/zh-Hans.lproj/Localizable.strings (1)</summary><blockquote>

`330-332`: LGTM!



Also applies to: 482-485

</blockquote></details>
<details>
<summary>ios/VaultSyncTests/SkipFamilyTests.swift (1)</summary><blockquote>

`8-103`: LGTM!

</blockquote></details>
<details>
<summary>go/bridge/conflicts.go (1)</summary><blockquote>

`260-343`: LGTM!

</blockquote></details>
<details>
<summary>go/bridge/conflicts_test.go (1)</summary><blockquote>

`345-512`: LGTM!

</blockquote></details>
<details>
<summary>ios/VaultSync/Services/SyncBridgeService.swift (1)</summary><blockquote>

`216-232`: LGTM!

</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Comment thread ios/VaultSync/Models/SkipFamily.swift
Comment thread ios/VaultSync/Services/SyncthingManager.swift Outdated
Comment thread ios/VaultSync/Views/ConflictDiffView.swift Outdated
Comment thread ios/VaultSync/Views/ContentView.swift Outdated
psimaker added 5 commits May 23, 2026 12:12
If currentSyncError != nil OR !isRunning OR foldersWithErrors is non-empty,
the dashboard's syncStatusText already shows a higher-priority state
('Error', 'Starting…', 'Sync Issue'). The reconnecting spinner +
'Restoring connection to N device(s)' caption used to render anyway,
producing an incoherent row (e.g. red 'Error' text with a teal reconnect
spinner). shouldShowReconnectingUI mirrors the syncStatusText cascade so
the reconnecting visuals only appear when reconnecting is actually the
status being reported.
A user-added .stignore entry like '*.tmp' or 'drafts/' would, via
SyncthingManager.conflictGlob, produce a derived glob ('*.sync-conflict-*',
'drafts.sync-conflict-*') that could coincidentally match another manual
entry and produce a spurious pairing. The result would be swipe-delete
removing both unrelated patterns at once.

Add isConcreteFilePathPattern guard that excludes lines containing
glob metacharacters (*, ?, [], {}, !) or a trailing slash. Apply in
both the pre-pass and the main emission loop. Three new regression
tests cover '*.tmp', 'drafts/', and 'note?.md'.
…upConflicts

Previously errors from removeConflictFilesForOriginal and rescanFolder
were silently swallowed: the call returned success even when on-disk
conflict copies couldn't be removed or the rescan failed. The user saw
a 'Skipping enabled' alert while the conflict actually still surfaced
on the home screen, with no signal what went wrong.

Now both bridge errors are mapped to a SyncUserError and returned to
the caller, so the existing 'Could not add filter' alert path
surfaces a real message. The .stignore write still happens before the
cleanup, so on partial failure the file is at least correctly
ignored — the error message tells the user the leftover copies
weren't removed.
…s immediately

Honest scope note: this only wraps the call in a Task and returns the
button handler synchronously. The underlying skipFileAndCleanupConflicts
is still @MainActor-isolated (it touches manager state, refreshConflicts,
etc.) so the file I/O continues to run on the main actor. A full move
to a background executor would require splitting the bridge cleanup,
rescan, and refresh paths into nonisolated entry points — out of scope
for this PR. The Task wrapper is a non-regressing first step and makes
that future refactor strictly easier.
@psimaker

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@psimaker

Copy link
Copy Markdown
Owner Author

@coderabbitai resolve

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
✅ Actions performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

@psimaker
psimaker merged commit 9c36fbd into main May 23, 2026
6 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.

Issue with ignoring files

1 participant