Repository navigation
fix(backup): recheck opt-out before publishing - #1379
Merged
Merged
Conversation
Cancel active backup work when local backup is disabled or stopped, and recheck opt-out and abort state after createBackup resolves before publishing or pruning. Fixes #1372
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
khaira777
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix the P2 release-readiness issue from PRs #1303, #1329, and #1331 (#1331) at main/services/local-backup.ts:218-236: recheck backup opt-out before publishing an in-flight snapshot. In main/services/local-backup.ts, backupIfDue() checks isLocalBackupEnabled() only before awaiting createBackup(staged, this.abortController.signal). If local backups are disabled while createBackup is in flight, backupIfDue() still publishes the snapshot and prunes existing backups. Cancel active backup work when local backup is disabled or stopped, and recheck opt-out and abort state after createBackup resolves before publishing or pruning, covered by a controlled delayed-backup test.
Fixes #1372
What Changed
LocalBackupServicework vialocalBackup.cancel()whensetLocalBackupEnabled(false)is called.this.stopping,this.abortController?.signal.aborted, and!isLocalBackupEnabled()aftercreateBackup()resolves inbackupIfDue(), cleaning up the staged snapshot and returning before publishing or pruning.tests/local-backup.test.tscovering in-flight opt-out, post-flight opt-out and abort signal rechecks, and service stop duringcreateBackup().Risk Assessment
✅ Low: The change is a small, well-bounded fix that cancels in-flight local backups on opt-out, rechecks opt-out and abort/stopping state after createBackup resolves before publishing or pruning, removes the unused abort() alias from Round 1, and adds deterministic delayed-backup tests.
Testing
Built the backend (
npm run build), ran thenpm run test:database-tools-apiandnpm run test:local-backupsuites (expanded to also test!isLocalBackupEnabled()andabortController.signal.abortedpost-createBackuprechecks independently), and drove five live scenarios against the compiled Express server and SQLite database in an isolated environment (live-local-backup-optout-evidence.json). This change is a backend service fix with no UI surface change, so evidence was captured as structured API, SQLite, and filesystem state artifacts. All checks and live scenarios passed.npm run test:local-backupnpm run test:local-backupnpm run test:local-backupnpm run test:local-backupEvidence: Live local backup opt-out and abort verification evidence
{ "scenario2_unmocked_createBackup_abort_on_opt_out": { "maintenanceStarted": true, "enabledAtMaintenanceStart": true, "statusAfterOptOut": { "enabled": false, "retention": 5, "lastBackupAt": "2026-01-05T03:00:00.000Z" }, "persistedSettingRow": { "key": "local_backup_enabled", "value": "false" }, "leftoverStagingFiles": [] }, "scenario3_post_createBackup_recheck_on_http_opt_out": { "stagedPath": ".flo-backup-97b478a6-3ebe-4b8a-8db6-7fb587c65047.tmp", "signalAbortedImmediatelyOnHttpPut": true, "httpPutStatus": 200, "stagedFileExistsAfterResolution": false, "leftoverStagingFiles": [] }, "scenario4_stop_service_during_in_flight_backup": { "stagedPath": ".flo-backup-1e0135c3-d507-4a60-aaa1-5e8a93dad847.tmp", "signalAbortedOnStop": true, "stagedFileExistsAfterStop": false }, "scenario5_independent_post_flight_guards": { "dbOnlyOptOut": { "signalAborted": false, "isLocalBackupEnabledAtResolve": false, "stagedRemoved": true, "backupsUnchanged": true }, "disableThenReenableDuringFlight": { "signalAborted": true, "isLocalBackupEnabledAtResolve": true, "stagedRemoved": true, "backupsUnchanged": true } } }Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
main/services/local-backup.ts:214-LocalBackupService.abort()is an unused alias forLocalBackupService.cancel()with no callers in the codebase or tests. The user intent requires cancelling active backup work when local backup is disabled or stopped, whichsetLocalBackupEnabled(false)andstop()already do vialocalBackup.cancel()/this.abortController?.abort(). Remove the unrequiredabort()alias method.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
npm run test:local-backupnpm run test:local-backupnpm run test:local-backupnpm run test:local-backupnpm run buildnpm run test:database-tools-apinpm run test:local-backupELECTRON_RUN_AS_NODE=1 ./node_modules/.bin/electron -e '<live server & LocalBackupService scenario driver>'✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.