From 75a530e7fdea4f55f83e2dcd94053d093c17dbbe Mon Sep 17 00:00:00 2001 From: khaira777 <777gurkirat@gmail.com> Date: Mon, 26 Oct 2026 12:30:57 -0400 Subject: [PATCH 1/3] fix(backup): recheck opt-out before publishing 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 --- main/services/local-backup.ts | 15 ++++ tests/local-backup.test.ts | 162 ++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+) diff --git a/main/services/local-backup.ts b/main/services/local-backup.ts index 78e2478e0..efcd8984e 100644 --- a/main/services/local-backup.ts +++ b/main/services/local-backup.ts @@ -29,6 +29,9 @@ export function isLocalBackupEnabled(): boolean { export function setLocalBackupEnabled(enabled: boolean): void { upsertSettings({ [LOCAL_BACKUP_SETTING_KEY]: enabled ? 'true' : 'false' }); + if (!enabled) { + localBackup.cancel(); + } } export function localBackupFileName(at: Date): string { @@ -204,6 +207,14 @@ class LocalBackupService { await this.running?.catch(() => {}); } + cancel(): void { + this.abortController?.abort(); + } + + abort(): void { + this.cancel(); + } + runIfDue(): Promise { if (this.running || this.stopping) return this.running ?? Promise.resolve(); this.running = this.backupIfDue() @@ -227,6 +238,10 @@ class LocalBackupService { let name: string; try { await createBackup(staged, this.abortController.signal); + if (this.stopping || this.abortController?.signal.aborted || !isLocalBackupEnabled()) { + removeQuietly(staged); + return; + } name = publishBackup(staged, dir); } catch (error) { removeQuietly(staged); diff --git a/tests/local-backup.test.ts b/tests/local-backup.test.ts index 562202e69..cabd7675c 100644 --- a/tests/local-backup.test.ts +++ b/tests/local-backup.test.ts @@ -592,6 +592,168 @@ async function main(): Promise { await localBackup.runIfDue(); assert.deepEqual(listLocalBackups(backupDir), []); + // Controlled delayed createBackup: disabling local backups while createBackup is in flight + // cancels active work, cleans up the staged file, does not publish to the backup directory, + // and does not prune existing backups. + ensureOwner(); + for (const name of listLocalBackups(backupDir)) { + fs.unlinkSync(path.join(backupDir, name)); + } + const existingRetentionBackups: string[] = []; + for (let day = 1; day <= LOCAL_BACKUP_RETENTION; day++) { + const name = localBackupFileName(new Date(Date.UTC(2026, 0, day, 3, 0, 0))); + fs.writeFileSync(path.join(backupDir, name), `retention-${day}`); + existingRetentionBackups.push(name); + } + const existingBackupsBefore = listLocalBackups(backupDir); + assert.equal(existingBackupsBefore.length, LOCAL_BACKUP_RETENTION); + + setLocalBackupEnabled(true); + assert.equal(isLocalBackupEnabled(), true); + + const dbModule = require('../main/db'); + const originalCreateBackup = dbModule.createBackup; + let stagedPathSeen: string | null = null; + let abortSignalSeen: AbortSignal | null = null; + let releaseDelayedBackup!: () => void; + const backupReleasePromise = new Promise((resolve) => { + releaseDelayedBackup = resolve; + }); + let notifyDelayedBackupStarted!: () => void; + const backupStartedPromise = new Promise((resolve) => { + notifyDelayedBackupStarted = resolve; + }); + + dbModule.createBackup = async ( + targetPath?: string, + signal?: AbortSignal, + _options?: any, + ) => { + stagedPathSeen = targetPath ?? null; + abortSignalSeen = signal ?? null; + notifyDelayedBackupStarted(); + await backupReleasePromise; + if (targetPath) { + fs.writeFileSync(targetPath, 'delayed-staged-content'); + } + return { path: targetPath ?? '', schemaVersion: 118 }; + }; + + try { + const inFlightRun = localBackup.runIfDue(); + await backupStartedPromise; + assert.ok(stagedPathSeen, 'delayed createBackup received staged path'); + + // While createBackup is still awaiting completion, disable local backups via settings update (opt-out) + const optOutRes = await request(api) + .put('/api/db-tools/local-backup') + .send({ enabled: false }); + assert.equal(optOutRes.status, 200); + assert.equal(optOutRes.body.enabled, false); + assert.equal(isLocalBackupEnabled(), false, 'local backup disabled during in-flight backup'); + assert.equal( + abortSignalSeen?.aborted, + true, + 'in-flight backup is signalled to abort when local backup is disabled', + ); + + // Allow createBackup to finish + releaseDelayedBackup(); + await inFlightRun; + + // Assert that the staged file is cleaned up, NO backup is published, and existing backups are not pruned + assert.equal( + fs.existsSync(stagedPathSeen!), + false, + 'staged backup file is cleaned up after opt-out', + ); + const backupsAfterFlight = listLocalBackups(backupDir); + assert.deepEqual( + backupsAfterFlight, + existingBackupsBefore, + 'no backup file is published and pruneLocalBackups does not delete existing backups', + ); + const leftoverTempFiles = fs + .readdirSync(backupDir) + .filter((name) => name.startsWith('.')); + assert.deepEqual(leftoverTempFiles, [], 'no temporary files left behind in backup dir'); + } finally { + dbModule.createBackup = originalCreateBackup; + } + + // Controlled delayed createBackup: stopping the service while createBackup is in flight + // aborts active work, cleans up the staged file, and does not publish. + ensureOwner(); + for (const name of listLocalBackups(backupDir)) { + fs.unlinkSync(path.join(backupDir, name)); + } + for (let day = 1; day <= LOCAL_BACKUP_RETENTION; day++) { + const name = localBackupFileName(new Date(Date.UTC(2026, 0, day, 3, 0, 0))); + fs.writeFileSync(path.join(backupDir, name), `retention-${day}`); + } + const existingBeforeStop = listLocalBackups(backupDir); + assert.equal(existingBeforeStop.length, LOCAL_BACKUP_RETENTION); + setLocalBackupEnabled(true); + + let stagedPathSeenStop: string | null = null; + let abortSignalSeenStop: AbortSignal | null = null; + let releaseDelayedBackupStop!: () => void; + const backupReleasePromiseStop = new Promise((resolve) => { + releaseDelayedBackupStop = resolve; + }); + let notifyDelayedBackupStartedStop!: () => void; + const backupStartedPromiseStop = new Promise((resolve) => { + notifyDelayedBackupStartedStop = resolve; + }); + + dbModule.createBackup = async ( + targetPath?: string, + signal?: AbortSignal, + _options?: any, + ) => { + stagedPathSeenStop = targetPath ?? null; + abortSignalSeenStop = signal ?? null; + notifyDelayedBackupStartedStop(); + await backupReleasePromiseStop; + if (targetPath) { + fs.writeFileSync(targetPath, 'delayed-staged-content-stop'); + } + return { path: targetPath ?? '', schemaVersion: 118 }; + }; + + try { + const inFlightRunStop = localBackup.runIfDue(); + await backupStartedPromiseStop; + assert.ok(stagedPathSeenStop, 'delayed createBackup received staged path before stop'); + + const stopPromise = localBackup.stop(); + assert.equal(abortSignalSeenStop?.aborted, true, 'stop aborts in-flight backup signal'); + + // Allow createBackup to finish while stop() is awaiting running + releaseDelayedBackupStop(); + await stopPromise; + await inFlightRunStop; + + assert.equal( + fs.existsSync(stagedPathSeenStop!), + false, + 'staged backup file is cleaned up after stop()', + ); + const backupsAfterStop = listLocalBackups(backupDir); + assert.deepEqual( + backupsAfterStop, + existingBeforeStop, + 'no backup file is published when stopped during createBackup', + ); + assert.deepEqual( + fs.readdirSync(backupDir).filter((name) => name.startsWith('.')), + [], + 'no temporary files left behind in backup dir after stop', + ); + } finally { + dbModule.createBackup = originalCreateBackup; + } + await localBackup.stop(); closeDatabase(); fs.rmSync(testDir, { recursive: true, force: true }); From d067413aaeb7ac0fd40e31034090a5c922b16ed6 Mon Sep 17 00:00:00 2001 From: khaira777 <777gurkirat@gmail.com> Date: Mon, 26 Oct 2026 12:40:39 -0400 Subject: [PATCH 2/3] no-mistakes(review): {"summary":"remove unused LocalBackupService.abort alias"} --- main/services/local-backup.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/main/services/local-backup.ts b/main/services/local-backup.ts index efcd8984e..c1bd97e74 100644 --- a/main/services/local-backup.ts +++ b/main/services/local-backup.ts @@ -211,10 +211,6 @@ class LocalBackupService { this.abortController?.abort(); } - abort(): void { - this.cancel(); - } - runIfDue(): Promise { if (this.running || this.stopping) return this.running ?? Promise.resolve(); this.running = this.backupIfDue() From 8f36d7acc0017c62930342170667557c8ab73473 Mon Sep 17 00:00:00 2001 From: khaira777 <777gurkirat@gmail.com> Date: Mon, 26 Oct 2026 13:03:26 -0400 Subject: [PATCH 3/3] no-mistakes(document): style: format local backup opt-out tests with oxfmt --- tests/local-backup.test.ts | 108 +++++++++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 16 deletions(-) diff --git a/tests/local-backup.test.ts b/tests/local-backup.test.ts index cabd7675c..d970b16f6 100644 --- a/tests/local-backup.test.ts +++ b/tests/local-backup.test.ts @@ -624,11 +624,7 @@ async function main(): Promise { notifyDelayedBackupStarted = resolve; }); - dbModule.createBackup = async ( - targetPath?: string, - signal?: AbortSignal, - _options?: any, - ) => { + dbModule.createBackup = async (targetPath?: string, signal?: AbortSignal, _options?: any) => { stagedPathSeen = targetPath ?? null; abortSignalSeen = signal ?? null; notifyDelayedBackupStarted(); @@ -645,9 +641,7 @@ async function main(): Promise { assert.ok(stagedPathSeen, 'delayed createBackup received staged path'); // While createBackup is still awaiting completion, disable local backups via settings update (opt-out) - const optOutRes = await request(api) - .put('/api/db-tools/local-backup') - .send({ enabled: false }); + const optOutRes = await request(api).put('/api/db-tools/local-backup').send({ enabled: false }); assert.equal(optOutRes.status, 200); assert.equal(optOutRes.body.enabled, false); assert.equal(isLocalBackupEnabled(), false, 'local backup disabled during in-flight backup'); @@ -673,10 +667,96 @@ async function main(): Promise { existingBackupsBefore, 'no backup file is published and pruneLocalBackups does not delete existing backups', ); - const leftoverTempFiles = fs - .readdirSync(backupDir) - .filter((name) => name.startsWith('.')); + const leftoverTempFiles = fs.readdirSync(backupDir).filter((name) => name.startsWith('.')); assert.deepEqual(leftoverTempFiles, [], 'no temporary files left behind in backup dir'); + + // Recheck !isLocalBackupEnabled() independently of abortSignal.aborted: + // if local_backup_enabled becomes 'false' in the database while createBackup is in flight + // without triggering abortController.abort(), the post-createBackup recheck still discards + // the staged snapshot without publishing or pruning. + setLocalBackupEnabled(true); + stagedPathSeen = null; + abortSignalSeen = null; + const backupReleaseOptOutOnly = new Promise((resolve) => { + releaseDelayedBackup = resolve; + }); + const backupStartedOptOutOnly = new Promise((resolve) => { + notifyDelayedBackupStarted = resolve; + }); + dbModule.createBackup = async (targetPath?: string, signal?: AbortSignal, _options?: any) => { + stagedPathSeen = targetPath ?? null; + abortSignalSeen = signal ?? null; + notifyDelayedBackupStarted(); + await backupReleaseOptOutOnly; + if (targetPath) { + fs.writeFileSync(targetPath, 'delayed-staged-optout-only'); + } + return { path: targetPath ?? '', schemaVersion: 118 }; + }; + + const inFlightOptOutOnly = localBackup.runIfDue(); + await backupStartedOptOutOnly; + getDatabase() + .prepare( + "INSERT OR REPLACE INTO settings (key, value, updated_at) VALUES ('local_backup_enabled', 'false', ?)", + ) + .run(now()); + assert.equal(isLocalBackupEnabled(), false); + assert.equal(abortSignalSeen!.aborted, false, 'signal was not aborted for direct DB opt-out'); + releaseDelayedBackup(); + await inFlightOptOutOnly; + assert.equal( + fs.existsSync(stagedPathSeen!), + false, + 'staged file removed on post-flight opt-out recheck', + ); + assert.deepEqual( + listLocalBackups(backupDir), + existingBackupsBefore, + 'post-flight !isLocalBackupEnabled() recheck prevents publishing and pruning', + ); + + // Recheck abortSignal.aborted independently of !isLocalBackupEnabled(): + // if local backup is disabled (aborting the signal) and then re-enabled while the aborted + // createBackup is still unwinding, the aborted snapshot is still discarded. + setLocalBackupEnabled(true); + stagedPathSeen = null; + abortSignalSeen = null; + const backupReleaseAbortOnly = new Promise((resolve) => { + releaseDelayedBackup = resolve; + }); + const backupStartedAbortOnly = new Promise((resolve) => { + notifyDelayedBackupStarted = resolve; + }); + dbModule.createBackup = async (targetPath?: string, signal?: AbortSignal, _options?: any) => { + stagedPathSeen = targetPath ?? null; + abortSignalSeen = signal ?? null; + notifyDelayedBackupStarted(); + await backupReleaseAbortOnly; + if (targetPath) { + fs.writeFileSync(targetPath, 'delayed-staged-abort-only'); + } + return { path: targetPath ?? '', schemaVersion: 118 }; + }; + + const inFlightAbortOnly = localBackup.runIfDue(); + await backupStartedAbortOnly; + setLocalBackupEnabled(false); + assert.equal(abortSignalSeen!.aborted, true, 'disabling aborts in-flight signal'); + setLocalBackupEnabled(true); + assert.equal(isLocalBackupEnabled(), true, 're-enabled before in-flight backup finishes'); + releaseDelayedBackup(); + await inFlightAbortOnly; + assert.equal( + fs.existsSync(stagedPathSeen!), + false, + 'staged file removed on post-flight abort recheck', + ); + assert.deepEqual( + listLocalBackups(backupDir), + existingBackupsBefore, + 'post-flight abortSignal.aborted recheck prevents publishing and pruning', + ); } finally { dbModule.createBackup = originalCreateBackup; } @@ -706,11 +786,7 @@ async function main(): Promise { notifyDelayedBackupStartedStop = resolve; }); - dbModule.createBackup = async ( - targetPath?: string, - signal?: AbortSignal, - _options?: any, - ) => { + dbModule.createBackup = async (targetPath?: string, signal?: AbortSignal, _options?: any) => { stagedPathSeenStop = targetPath ?? null; abortSignalSeenStop = signal ?? null; notifyDelayedBackupStartedStop();