fix(ios): adopt background-started engine at onboarding, surface engine death honestly (#61) - #63
Conversation
…ne death honestly (#61) Closes #61 — the #60 follow-up bundle, all three points. Onboarding completion guarded on the manager alone: with an engine already running from a background wake-up, start() mapped the Go floor's "already running" rejection to a user-facing error in the first-run moment. The completion handler now routes through the same three-way decision as scene activation (cold start / adopt / already attached) and keeps the #56 reconcile in the already-attached branch. Covered by tests that run start() and adoptRunningEngine() against a real background-started engine. The residual #60 adoption race (a background stop whose lock re-read passed just before the foreground claimed) left the manager polling a dead engine: empty folder JSON rendered as a healthy "Ready" while nothing synced. The poll loop now detects the dead bridge, resets scene-style, and cold-starts once per externally initiated generation; a second death in the same generation stays stopped and surfaces an honest error instead of flapping a crash-looping engine (decision 009). The restart reconciles against the last known root so accepts do not stay held until the next scene cycle (decision 008). The silent-push lifecycle guards (fast-path rescan, lifecycle ownership, forced restart) moved behind injectable seams (BackgroundSyncGuards). Behavior is unchanged; the #60 decision-time lock re-read is now pinned by tests, including the foreground-adoption-mid-push race. The unit-test host app no longer manages the process-global engine lifecycle (TestHost guard, decision 010): its own scene handler would otherwise auto-restart engines a test deliberately stopped — found as a live flake by the new death-detection tests, reproducible with a single test running solo. Bridge-mutating suites are serialized under the new EngineBridgeSuites umbrella for the same reason. Verified on macOS: full Xcode suite green (202 tests, iPhone 17 Pro simulator), EngineBridgeSuites stress-run over 3 iterations green, design-token lint green, localization key counts identical across all four languages (new strings in en/de/es/zh-Hans). No go/ changes — no xcframework rebuild needed.
📝 WalkthroughWalkthroughAdds engine-death detection with a once-per-generation auto-restart budget in SyncthingManager, refactors BackgroundSyncService's lifecycle decisions into a testable BackgroundSyncGuards abstraction, introduces a TestHost guard so unit tests never manage the embedded engine, and adds corresponding localization strings, ADR docs, and tests. ChangesEngine lifecycle recovery and test isolation
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)
✨ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/VaultSync/Views/OnboardingView.swift (1)
83-90: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
onAppearshould adopt a running engine instead of always starting one
OnboardingView.onAppearstill callssyncthingManager.start()directly, so if a BG refresh/processing or silent-push sync starts the bridge while the user is still onboarding, returning to the app can hit the Go “already running” error again.Route this through the same
sceneActivationAction/adoptRunningEngine()path used inVaultSyncApp.swift, or guard on the bridge state before callingstart().🤖 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/Views/OnboardingView.swift` around lines 83 - 90, OnboardingView.onAppear is still starting Syncthing directly via syncthingManager.start(), which can collide with an already-running bridge started by background sync. Update the onAppear flow to use the same sceneActivationAction/adoptRunningEngine path as VaultSyncApp, or check the bridge state before starting so it adopts an existing engine instead of triggering the “already running” error.Source: Learnings
🧹 Nitpick comments (1)
ios/VaultSync/App/VaultSyncApp.swift (1)
149-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated engine-attach logic between scene-activation and onboarding-completion.
The
.coldStartand.adoptRunningEnginebodies here are identical to the.activescene-phase branch above (Lines 77-106). Two copies of the same reset/adopt/fallback sequence will drift if one is updated (e.g. the next engine-death fix) without the other.Consider extracting a shared helper (e.g.
attachToEngine(action:)) that bothonChangeclosures call, keeping only the differing tail (.alreadyAttached) at each call site.♻️ Sketch of a shared helper
+ private func attachToEngine(_ action: BackgroundSyncService.SceneActivationAction) { + switch action { + case .coldStart: + if syncthingManager.isRunning { + syncthingManager.resetForRestart() + } + vaultManager.restoreAccess() + Task { + await syncthingManager.start() + syncthingManager.reconcileFolderPaths(obsidianRoot: vaultManager.obsidianBasePath) + } + case .adoptRunningEngine: + vaultManager.restoreAccess() + if syncthingManager.adoptRunningEngine() { + syncthingManager.reconcileFolderPaths(obsidianRoot: vaultManager.obsidianBasePath) + } else { + Task { + await syncthingManager.start() + syncthingManager.reconcileFolderPaths(obsidianRoot: vaultManager.obsidianBasePath) + } + } + case .alreadyAttached: + break // caller handles this case + } + }🤖 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/App/VaultSyncApp.swift` around lines 149 - 182, Extract the duplicated reset/adopt/fallback engine-attach flow from the onboarding-completion path in VaultSyncApp into the same shared helper used by the scene-activation branch, so both call sites reuse one implementation instead of maintaining identical `.coldStart` and `.adoptRunningEngine` logic. Locate the repeated `switch BackgroundSyncService.sceneActivationAction(...)` handling around the `completed` check and move the common `syncthingManager.resetForRestart()`, `vaultManager.restoreAccess()`, `syncthingManager.adoptRunningEngine()`, and fallback `Task { await syncthingManager.start() ... }` sequence into a helper such as `attachToEngine(action:)`, leaving only the branch-specific `.alreadyAttached` behavior at each caller.
🤖 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.
Outside diff comments:
In `@ios/VaultSync/Views/OnboardingView.swift`:
- Around line 83-90: OnboardingView.onAppear is still starting Syncthing
directly via syncthingManager.start(), which can collide with an already-running
bridge started by background sync. Update the onAppear flow to use the same
sceneActivationAction/adoptRunningEngine path as VaultSyncApp, or check the
bridge state before starting so it adopts an existing engine instead of
triggering the “already running” error.
---
Nitpick comments:
In `@ios/VaultSync/App/VaultSyncApp.swift`:
- Around line 149-182: Extract the duplicated reset/adopt/fallback engine-attach
flow from the onboarding-completion path in VaultSyncApp into the same shared
helper used by the scene-activation branch, so both call sites reuse one
implementation instead of maintaining identical `.coldStart` and
`.adoptRunningEngine` logic. Locate the repeated `switch
BackgroundSyncService.sceneActivationAction(...)` handling around the
`completed` check and move the common `syncthingManager.resetForRestart()`,
`vaultManager.restoreAccess()`, `syncthingManager.adoptRunningEngine()`, and
fallback `Task { await syncthingManager.start() ... }` sequence into a helper
such as `attachToEngine(action:)`, leaving only the branch-specific
`.alreadyAttached` behavior at each caller.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 547cf384-9791-483b-af4f-db53197f773a
📒 Files selected for processing (19)
CHANGELOG.mddocs/decisions/009-engine-death-restart-once.mddocs/decisions/010-test-host-never-manages-engine.mdios/VaultSync/App/TestHost.swiftios/VaultSync/App/VaultSyncApp.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/Views/OnboardingView.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/es.lproj/Localizable.stringsios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSyncTests/BackgroundSyncGuardsTests.swiftios/VaultSyncTests/EngineBridgeSuites.swiftios/VaultSyncTests/EngineDeathDetectionTests.swiftios/VaultSyncTests/OnboardingEngineAttachTests.swiftios/VaultSyncTests/SceneActivationAdoptionTests.swiftios/VaultSyncTests/TestSupport.swift
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Build & Test
⚠️ CI failures not shown inline (2)
GitHub Actions: Security / Go Lint: fix(ios): adopt background-started engine at onboarding, surface
engine death honestly (#61)
Conclusion: failure
##[group]Run unformatted="$(gofmt -l go/bridge notify)"
�[36;1munformatted="$(gofmt -l go/bridge notify)"�[0m
�[36;1mif [ -n "$unformatted" ]; then�[0m
�[36;1m echo "::error::These files need gofmt:"�[0m
GitHub Actions: Security / 1_Go Lint.txt: fix(ios): adopt background-started engine at onboarding, surface
engine death honestly (#61)
Conclusion: failure
##[group]Run unformatted="$(gofmt -l go/bridge notify)"
�[36;1munformatted="$(gofmt -l go/bridge notify)"�[0m
�[36;1mif [ -n "$unformatted" ]; then�[0m
�[36;1m echo "::error::These files need gofmt:"�[0m
🧰 Additional context used
📓 Path-based instructions (9)
**/*.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/VaultSyncTests/EngineBridgeSuites.swiftios/VaultSync/App/TestHost.swiftios/VaultSyncTests/TestSupport.swiftios/VaultSync/Views/OnboardingView.swiftios/VaultSyncTests/OnboardingEngineAttachTests.swiftios/VaultSyncTests/BackgroundSyncGuardsTests.swiftios/VaultSyncTests/SceneActivationAdoptionTests.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSyncTests/EngineDeathDetectionTests.swiftios/VaultSync/App/VaultSyncApp.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/VaultSyncTests/EngineBridgeSuites.swiftios/VaultSync/App/TestHost.swiftios/VaultSyncTests/TestSupport.swiftios/VaultSync/Views/OnboardingView.swiftios/VaultSyncTests/OnboardingEngineAttachTests.swiftios/VaultSyncTests/BackgroundSyncGuardsTests.swiftios/VaultSyncTests/SceneActivationAdoptionTests.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSyncTests/EngineDeathDetectionTests.swiftios/VaultSync/App/VaultSyncApp.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/VaultSyncTests/EngineBridgeSuites.swiftios/VaultSync/App/TestHost.swiftios/VaultSyncTests/TestSupport.swiftios/VaultSync/Views/OnboardingView.swiftios/VaultSyncTests/OnboardingEngineAttachTests.swiftios/VaultSyncTests/BackgroundSyncGuardsTests.swiftios/VaultSyncTests/SceneActivationAdoptionTests.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSyncTests/EngineDeathDetectionTests.swiftios/VaultSync/App/VaultSyncApp.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/VaultSyncTests/EngineBridgeSuites.swiftios/VaultSync/App/TestHost.swiftios/VaultSyncTests/TestSupport.swiftios/VaultSync/Views/OnboardingView.swiftios/VaultSyncTests/OnboardingEngineAttachTests.swiftios/VaultSyncTests/BackgroundSyncGuardsTests.swiftios/VaultSyncTests/SceneActivationAdoptionTests.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSyncTests/EngineDeathDetectionTests.swiftios/VaultSync/App/VaultSyncApp.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/VaultSyncTests/EngineBridgeSuites.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/App/TestHost.swiftios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSyncTests/TestSupport.swiftios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/Views/OnboardingView.swiftios/VaultSyncTests/OnboardingEngineAttachTests.swiftios/VaultSyncTests/BackgroundSyncGuardsTests.swiftios/VaultSyncTests/SceneActivationAdoptionTests.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSyncTests/EngineDeathDetectionTests.swiftios/VaultSync/App/VaultSyncApp.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/es.lproj/Localizable.strings
**
⚙️ 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/VaultSyncTests/EngineBridgeSuites.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/App/TestHost.swiftios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSyncTests/TestSupport.swiftios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/Views/OnboardingView.swiftios/VaultSyncTests/OnboardingEngineAttachTests.swiftdocs/decisions/010-test-host-never-manages-engine.mdCHANGELOG.mdios/VaultSyncTests/BackgroundSyncGuardsTests.swiftdocs/decisions/009-engine-death-restart-once.mdios/VaultSyncTests/SceneActivationAdoptionTests.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSyncTests/EngineDeathDetectionTests.swiftios/VaultSync/App/VaultSyncApp.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/es.lproj/Localizable.strings
**/*
⚙️ 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/VaultSyncTests/EngineBridgeSuites.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/App/TestHost.swiftios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSyncTests/TestSupport.swiftios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/Views/OnboardingView.swiftios/VaultSyncTests/OnboardingEngineAttachTests.swiftdocs/decisions/010-test-host-never-manages-engine.mdCHANGELOG.mdios/VaultSyncTests/BackgroundSyncGuardsTests.swiftdocs/decisions/009-engine-death-restart-once.mdios/VaultSyncTests/SceneActivationAdoptionTests.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSyncTests/EngineDeathDetectionTests.swiftios/VaultSync/App/VaultSyncApp.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/es.lproj/Localizable.strings
docs/**
⚙️ CodeRabbit configuration file
docs/**: # ArchitectureVaultSync embeds Syncthing's Go reference implementation as an iOS library via gomobile — no reimplementation of the protocol in Swift, and guaranteed wire compatibility.
┌─────────────────────────────────┐ │ SwiftUI Frontend │ iOS-native UI, Swift 6 ├─────────────────────────────────┤ │ Swift ↔ Go Bridge │ thin API via gomobile │ │ → exported as .xcframework ├─────────────────────────────────┤ │ syncthing/lib (Go) │ protocol, discovery, sync └─────────────────────────────────┘ ↕ filesystem ┌─────────────────────────────────┐ │ Obsidian Vault (direct) │ Obsidian's iOS sandbox └─────────────────────────────────┘🔄 Sync strategy
- Foreground — Syncthing runs unrestricted: immediate, continuous sync.
- Background —
BGAppRefreshTask(requested ~15 min out; iOS decides the actual timing) +BGProcessingTask(overnight catch-up: multi-minute budget while charging with network) +BGContinuedProcessingTask(iOS 26+, longer runtime for user-initiated tasks). A ~30s grace window after backgrounding lets in-flight work finish.- Push (Cloud Relay) — optional. Near-realtime
server → iPhonewake-ups via APNs silent push. See relay-spec.md.VaultSync is intentionally asymmetric:
Direction Path Server → iPhone vaultsync-notifyspots outgoing changes → Cloud Relay silent push → VaultSync wakes and pulls.iPhone → Server iOS doesn't guarantee timely background execution for local edits. The reliable path is to open VaultSync and let embedded Syncthing run in the foreground — a Shortcuts automation can do that automatically whenever you leave Obsidian. Cloud Relay is a
server → iPhoneacceleration path, not a guarantee of symmetric real-time background sync.Connection paths & iOS network privacy
How peers are reached, fastest f...
Files:
docs/decisions/010-test-host-never-manages-engine.mddocs/decisions/009-engine-death-restart-once.md
docs/decisions/**
⚙️ CodeRabbit configuration file
docs/decisions/**: # 001 — Two Syncthing folders never overlap on disk (three enforcement layers)Context: A server sharing more than one vault could hand the second share the same local folder as the first (1.6.0–1.7.0) or a subfolder inside an existing vault (1.7.1). Overlapping folders sync each other's content as their own — deleting the stray copy on any peer would have deleted the inner vault everywhere (
#45).Decision: The no-overlap invariant (equal, nested, or containing paths) is enforced in three independent layers, each with its own tests: the Go hard floor (
AcceptPendingFolder/AddFolderreject withfolderPathOverlapError), the Swift mapping (VaultManager.resolveSharePathreturnsnilrather than an overlapping path), and the launch shield (PathCollisionGuardpauses already-overlapping folders exactly once).Why: The single-layer version failed twice — both
#45bugs lived in the Swift mapping. The Go floor backstops future mapping bugs, the Swift layer turns a hard engine error into user guidance, and only the shield catches damage that predates the fix.Rejected alternative: Enforcing only in the Swift mapping — the proven failure mode; a bug there would silently re-open the hole with no backstop.
Links:
#45, PR#47(same-folder merge, 1.7.1), PR#51(nesting, 1.7.2).
docs/decisions/**: # 002 — Recovery from data damage is never automaticContext: When VaultSync detects existing damage (two vaults merged into one folder, or one nested inside another,
#45), an automatic repair would have to move, rename, delete, or re-accept user data whose true state only the user knows — after a merge, the app cannot tell which files belong to which vault.Decision: Recovery is always: pause the affected folders (exactly once — a folder the user deliberately resumes is never re-paused), explain the problem in a critical issue with concrete recovery steps, and let the user act.
Why: Sync propagates every local act...
Files:
docs/decisions/010-test-host-never-manages-engine.mddocs/decisions/009-engine-death-restart-once.md
**/*.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:
docs/decisions/010-test-host-never-manages-engine.mdCHANGELOG.mddocs/decisions/009-engine-death-restart-once.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-07T15:13:56.012Z
Learning: When the poll loop detects a dead bridge under an attached manager, reset it exactly like the scene cold-start path and automatically cold-start the engine once per externally initiated generation (`stop`, `resetForRestart`, `adoptRunningEngine` each grant a fresh budget; the auto-restart's own `start()` does not).
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-07T15:13:56.012Z
Learning: If the engine dies a second time in the same generation, leave it stopped and surface a user-visible error instead of retrying indefinitely.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-07T15:13:56.012Z
Learning: On an automatic restart, reconcile against the last known Obsidian root so accept decisions are not left pending until the next scene cycle.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-07T15:14:13.781Z
Learning: In app-level lifecycle code paths that start, adopt, or stop the engine (including the scene-activation handler, onboarding-completion handler, and `OnboardingView.onAppear`), bail out when `TestHost.isActive` so the unit-test host never manages the engine lifecycle.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-07T15:14:13.781Z
Learning: Any test suite that mutates bridge state must be declared inside the serialized `EngineBridgeSuites` umbrella so those suites never overlap each other.
📚 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/App/TestHost.swiftios/VaultSync/Views/OnboardingView.swiftios/VaultSync/Services/BackgroundSyncService.swiftios/VaultSync/Services/BackgroundSyncGuards.swiftios/VaultSync/App/VaultSyncApp.swiftios/VaultSync/Services/SyncthingManager.swift
🔇 Additional comments (19)
CHANGELOG.md (1)
23-24: LGTM!docs/decisions/009-engine-death-restart-once.md (1)
1-11: LGTM!docs/decisions/010-test-host-never-manages-engine.md (1)
1-11: LGTM!ios/VaultSync/de.lproj/Localizable.strings (1)
35-35: LGTM!Also applies to: 289-289, 310-310
ios/VaultSync/en.lproj/Localizable.strings (1)
35-35: LGTM!Also applies to: 289-289, 310-310
ios/VaultSync/es.lproj/Localizable.strings (1)
35-35: LGTM!Also applies to: 289-289, 310-310
ios/VaultSync/zh-Hans.lproj/Localizable.strings (1)
35-35: LGTM!Also applies to: 289-289, 310-310
ios/VaultSync/App/TestHost.swift (1)
1-13: LGTM!ios/VaultSync/App/VaultSyncApp.swift (1)
65-67: LGTM!ios/VaultSyncTests/SceneActivationAdoptionTests.swift (1)
4-92: LGTM — relocating into theEngineBridgeSuitesextension and dropping the inner.serializedis the correct pattern here: Swift Testing inherits a.serializedtrait from an outer suite into nested suites, so serialization is preserved as long asEngineBridgeSuitesitself carries.serialized(not in this file set, but consistent with the stated umbrella design). Test bodies are otherwise unchanged.ios/VaultSyncTests/OnboardingEngineAttachTests.swift (1)
1-68: 🎯 Functional CorrectnessLooks good.
ios/VaultSync/Services/BackgroundSyncGuards.swift (1)
1-81: LGTM!ios/VaultSync/Services/BackgroundSyncService.swift (1)
551-570: LGTM!Also applies to: 642-646
ios/VaultSyncTests/BackgroundSyncGuardsTests.swift (1)
1-104: LGTM!ios/VaultSync/Services/SyncthingManager.swift (2)
112-122: LGTM!Also applies to: 724-726, 744-744, 811-811, 1099-1117, 2255-2262
1026-1057: 🩺 Stability & AvailabilityNo issue here: duplicate starts are a benign handoff.
StartSyncthingis mutex-serialized and returnsalready running, and the manager’s restart path already has a running-engine attach flow, so a BGTask winning this window should just join the live bridge.> Likely an incorrect or invalid review comment.ios/VaultSyncTests/EngineBridgeSuites.swift (1)
1-11: LGTM!ios/VaultSyncTests/EngineDeathDetectionTests.swift (1)
1-117: LGTM!ios/VaultSyncTests/TestSupport.swift (1)
50-56: LGTM!
…per (#61) Addresses the CodeRabbit review on #63. It found a third consumer of the bridge-running/manager-cold state this PR fixes: OnboardingView.onAppear still called start() directly, so an engine started by a background handler before onboarding rendered could race the scene handler and flash the Go floor's "already running" as an error mid-onboarding. Instead of adding a third copy of the decide/reset/adopt/fallback sequence, all three call sites (scene activation, onboarding completion, onboarding appear) now go through EngineAttach.onForeground, which performs the cold-start or adoption plus the follow-up reconcile and returns the decision — only the legitimately different .alreadyAttached tails stay at the call sites (rescan debounce, #56 repeat reconcile, nothing). One copy means the next lifecycle fix cannot drift between consumers, and the wiring is unit-testable for the first time: new tests drive the helper against a real background-started engine (adopts without error, holds accepts per decision 008, leaves an attached manager untouched). Verified on macOS: full suite green (204 tests), EngineBridgeSuites stress-run over 3 iterations green, design-token lint green.
psimaker
changed the title
1 participant
Closes #61 — the #60 follow-up bundle, all three points:
onboarding completion adopts a background-started engine instead of
surfacing the Go floor's 'already running' as a user error; the poll loop
detects a dead bridge under an attached manager and restarts once per
generation instead of showing 'Ready' (decision 009); the silent-push
lifecycle guards moved behind injectable seams (BackgroundSyncGuards) with
the #60 decision-time lock re-read pinned by tests. Also: the unit-test host
no longer manages the process-global engine lifecycle (decision 010) — its
manager auto-restarted test-stopped engines mid-assertion. Verified: full
suite green on macOS (202 tests), bridge-suite stress-run over 3 iterations
green, design-token lint green, localization counts identical (new strings
in en/de/es/zh-Hans). No go/ changes — no xcframework rebuild needed.
BackgroundSyncGuards, including the decision-time lock re-read needed for the race covered by#60.en,de,es, andzh-Hans.Verification: full macOS test suite green, bridge-suite stress run green, design-token lint green, localization counts unchanged except for the new strings, and no
go/changes or xcframework rebuild needed.