fix(storage): propagate connection persistence failure (data-loss prevention)

[datlechin]

Summary

Fixes B1 from the 2026-05-09 full-app audit (docs/refactor/full-app-audit-2026-05-09/02-bugs-behavior.md). Severity: CRITICAL — data loss.

ConnectionStorage.saveConnections(_:) swallowed every persistence failure with a void return + Logger.error. Callers continued as if the write had succeeded, mutating dependent state (sync tracker, keychain, app settings) for a record that was never persisted.

Worst-case scenario before this PR

1. User triggers deleteConnection.
2. saveConnections throws (disk full, sandbox denied, encoding error).
3. The error is logged. The function returns Void.
4. Caller continues: syncTracker.markDeleted(.connection, id:) queues the connection for cloud removal.
5. Caller continues: deletePassword(for:) removes the keychain entry.
6. On next launch the connection record is still present on disk (the save failed). On next sync, the iCloud copy gets nuked. The keychain password is gone. The user's connection is now broken on every device.

A symmetric path existed in SyncRecordMapper.applyRemoteConnection (saved to disk; if it failed, the function reported success and the sync token advanced, dropping the remote update on the floor).

What changes

saveConnections now returns @discardableResult Bool. Every caller that mutates dependent state checks the result and aborts the downstream side effects on failure.

File	Site	Behavior on failure
ConnectionStorage.swift	addConnection	Return early; no markDirty, no password write
ConnectionStorage.swift	updateConnection	Return early; no markDirty, no password write
ConnectionStorage.swift	deleteConnection	Return early; no markDeleted, no password / SSH / TOTP / plugin-field removals
ConnectionStorage.swift	deleteConnections	Return early; no markDeleted for any of the batch
ConnectionStorage.swift	duplicateConnection	Return early; no markDirty, no password copy
ConnectionStorage.swift	migratePluginSecureFieldsIfNeeded	Log and retry on next launch (idempotent)
Core/Sync/SyncCoordinator.swift	applyRemoteChanges (deletions)	Log; the next pull will retry
Core/Sync/SyncCoordinator.swift	applyRemoteConnection	Return false so the sync token does not advance past this record
ConnectionFormCoordinator.swift	save (new + edit)	Set localized saveError, keep the form open, do not dismiss, do not fire connectionUpdated
ViewModels/WelcomeViewModel.swift	moveConnections, removeFromGroup, drag-reorder, group-reorder	Reload from disk to restore consistent state, no markDirty
Core/Storage/GroupStorage.swift	post-delete cascade clearing groupId	Log on failure

Diff stats

• 6 files, +72 / −17 LoC
• Sync-delete ordering invariant from CLAUDE.md ("persist first, then notify") is now enforced by the early-return guard, not by hope.

Test plan

Critical reproducers (the audit's worst case + the symmetric paths):

• Disk-full simulation: chflags uchg the connections file (or fill ~/Library/Containers/.../Documents with dd), trigger a delete from the welcome screen. Connection should remain in the list, password should remain in keychain, no CloudKit tombstone should be queued. Console log should show Aborted deleteConnection: persistence failed.
• Same with batch delete (multi-select 3 connections, delete). All three should remain.
• Same with add: open new connection form, fill it out, hit Save. Form should NOT dismiss; saveError shows the localized message; the user can correct disk space and retry.
• Same with edit: open existing connection form, change name, hit Save. Form should NOT dismiss.
• Same with duplicate: right-click → Duplicate. UI may briefly show the duplicate, but no keychain copy should fire and no markDirty should queue.
• Drag-reorder a connection across groups while the file is locked. UI should snap back to disk state on the next render.
• Cloud sync incoming connection update; lock the file before the pull. The sync token should NOT advance past the failed record (verified: applyRemoteConnection returns false). Re-unlock and re-pull → the record applies cleanly.

Non-regression:

• Normal connection CRUD continues to work end to end.
• CHANGELOG entry under Unreleased > Fixed.
• No new SwiftLint warnings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.