feat(local-backup): Track A safeguards for #7925 (#7932)

* fix(android): escape JS bridge args via JSONObject.quote (#7925)

`loadFromDb` interpolated the stored value into a single-quoted JS string
literal passed to `evaluateJavascript`. Beyond the security smell, this is
a real data-loss bug: `JSON.stringify` does not escape apostrophes, so a
backup blob containing one (e.g. a task titled "don't…") terminated the JS
literal and the load returned garbage — silently corrupting any restore
from `KeyValStore`.

Use `JSONObject.quote()` (already established in the file for
`emitForegroundServiceStartFailed`) for all three callback args, so values
containing `'`, `\`, newlines or `</script>` round-trip cleanly.

* feat(startup): log storage-persistence outcome on all branches (#7925)

`_requestPersistence()` was silent on native and on the `false` resolution
of `persist()`, so #7892-style "woke up blank" reports carried no signal
about whether the WebView store was actually persistent.

Always log `{persisted, granted, isNative, isElectron}` — including the
already-persisted branch, the persist-resolved branch (both true and false),
the error branch, and the no-`navigator.storage` branch. User-facing snack
gating is unchanged (still web-only, non-onboarding). Logging-only — no
behavioral change.

This is Track A1 in `docs/sync-and-op-log/sqlite-migration-followup.md`:
the diagnostics that decide whether the deeper protective steps
(near-empty write guard, SQLite migration) are worth the added complexity.

* docs(sync): refresh sqlite-migration followup after #7924 (#7925)

The followup backlog described the local-backup ring as TODO under A2,
but #7924 already shipped the periodic + app-private backup, two-generation
ring, empty-state write guard, and informed restore prompt. Bring the doc
in sync:

- Add the shipped local-backup work to "Where we are now".
- Replace the old A2 ("Periodic local auto-backup") with the narrower
  remaining gap: a debounced data-change backup trigger to complement the
  5-min timer.
- Add A3 (near-empty write-time overwrite guard) with a concrete starting
  threshold and a fail-safe rationale, sequenced after A1 so the
  diagnostics tune the threshold before it lands.
- New "Cross-cutting / hardening" section consolidating the items
  surfaced by the #7924 review (Kotlin JS-bridge escaping — now done;
  backup-date reader bridge for the restore prompt; robust restore on
  degraded boot; last-backup visibility; onboarding nudge for no-sync
  users).

* feat(local-backup): debounced on-data-change backup trigger (#7925)

The 5-min `interval()` was the only thing that drove `LocalBackupService._backup()`,
so a destructive event in the minutes before a WebView eviction could be lost
from the backup ring even though the live store had it.

Merge a `LOCAL_ACTIONS`-driven trigger into `_triggerBackupSave$` that fires
once after a 30s quiet period. Catches the typical "user made changes then
put phone down" pattern before the next periodic tick. `LOCAL_ACTIONS`
already filters out remote/hydration replays, and the existing empty-state
guard prevents writing a degraded post-eviction snapshot over a good
backup, so this strictly adds backup frequency — never spam.

Logic-level only — `_backup()` continues to early-return on non-target
platforms (web/PWA), so the trigger is safe to subscribe everywhere.

Closes A2 (remaining gap) in
`docs/sync-and-op-log/sqlite-migration-followup.md`.

* docs(sync): mark A1 + A2 shipped in sqlite-migration followup (#7925)

A1 (storage-persistence diagnostics) and A2 (debounced data-change backup
trigger) both landed this round. Update the suggested order and the A2
section so the doc accurately reflects what's left in Track A (just A3,
the near-empty write-time overwrite guard).

* feat(local-backup): near-empty write-time overwrite guard (A3, #7925)

The exact-empty guard in `_backup()` only catches a fully-degraded store.
The residual gap is a post-eviction boot that leaves the store near-empty,
the user adds 1-2 tasks before the 5-min timer fires, and the degraded
state then overwrites the good primary slot. The prev slot is still safe,
but the informed restore prompt only fires on a wholly fresh launch — so
without this guard the user has lost direct access to the better backup
until they uninstall/reinstall.

Add a per-platform near-empty guard in `_backupAndroid` / `_backupIOS`:
read the existing primary, compare task counts (active + young-archived +
old-archived via the new shared `countAllTasks` helper), and bail when a
< 3-task snapshot would clobber a >= 10-task existing backup. Electron is
unchanged — its rotated, timestamped backup chain isn't a single-slot
overwrite.

Threshold rationale: `summarizeBackupStr` counts archived tasks too, so
"near-empty" means the same thing on the read side (the restore prompt)
and the write side (this guard). Fail-safe — skipping a write only delays
capturing a real wipe, never loses data; the guard self-clears once the
store grows back past 3 tasks, so a legitimate bulk-delete is captured
on the next tick.

Marks Track A (#7925 / sqlite-migration-followup.md) complete.

* fix(android): JSONObject.quote() the sibling JS bridge callbacks (#7925)

`saveToDbCallback` / `removeFromDbCallback` / `clearDbCallback` still raw-
interpolated `requestId` into single-quoted JS string literals. The args are
nanoid strings today so it works — but only by caller hygiene. Mirror the
`loadFromDb` fix: quote all three so the bridge contract no longer depends
on what the caller happens to pass.

Compress the `loadFromDb` rationale comment in the process — the file-level
intent now lives on one line near the cluster.

* refactor(local-backup, startup): trim Track A code per multi-agent review

Two independent reviewers flagged the same set of cleanups on the Track A
commits (#7925). Applying the high-confidence ones:

- Drop `_escapeAndroidNewlines` + its two call sites. The Kotlin bridge fix
  (#7925, 663d747b4) now JSON-escapes newlines on the way out, so the JS-side
  workaround replaces nothing on any post-fix write. Removes the awkward
  "raw vs escaped" split in `_backupAndroid`.
- Hoist the A3 skip-and-log block into one private `_guardNearEmptyOverwrite`
  helper so the warn template can't drift between Android and iOS. Keep
  `_isNearEmptyOverwrite` as the pure predicate (the spec pins it).
- Tighten the A2 comment: `bulkApplyOperations` / `loadAllData` actually do
  transit `LOCAL_ACTIONS` (they aren't tagged `meta.isRemote`). Behaviour
  is still correct because the empty-state + A3 guards handle degraded data,
  but the previous comment overstated upstream filtering.
- Cut the multi-paragraph comment blocks around `DATA_CHANGE_BACKUP_DEBOUNCE`,
  the A3 constants, `_isNearEmptyOverwrite`, and the `_backup()` guard. Keep
  the issue refs; drop the prose that paraphrased the next line of code.
- Inline the `context` object spread in `_requestPersistence` — three log
  calls on adjacent lines didn't need a hoisted bag of fields.

No behavioural change. 21/21 local-backup specs + 18/18 startup specs green.
This commit is contained in:
Johannes Millan 2026-06-02 11:58:36 +02:00 committed by GitHub
parent 1e2260d0e2
commit 64d3219d3a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 515 additions and 120 deletions

View file

@ -93,29 +93,36 @@ class JavaScriptInterface(
@JavascriptInterface
fun saveToDb(requestId: String, key: String, value: String) {
(activity.application as App).keyValStore.set(key, value)
callJavaScriptFunction(FN_PREFIX + "saveToDbCallback('" + requestId + "')")
callJavaScriptFunction(FN_PREFIX + "saveToDbCallback(" + JSONObject.quote(requestId) + ")")
}
// #7925: quote every arg so a stored value with ' / \ / newlines / </script>
// can't break out of the JS literal. (JSON.stringify does not escape
// apostrophes — pre-fix, a backup blob with one silently corrupted the load.)
@Suppress("unused")
@JavascriptInterface
fun loadFromDb(requestId: String, key: String) {
val r = (activity.application as App).keyValStore.get(key, "")
// NOTE: ' are important as otherwise the json messes up
callJavaScriptFunction(FN_PREFIX + "loadFromDbCallback('" + requestId + "', '" + key + "', '" + r + "')")
callJavaScriptFunction(
FN_PREFIX + "loadFromDbCallback(" +
JSONObject.quote(requestId) + ", " +
JSONObject.quote(key) + ", " +
JSONObject.quote(r) + ")"
)
}
@Suppress("unused")
@JavascriptInterface
fun removeFromDb(requestId: String, key: String) {
(activity.application as App).keyValStore.set(key, null)
callJavaScriptFunction(FN_PREFIX + "removeFromDbCallback('" + requestId + "')")
callJavaScriptFunction(FN_PREFIX + "removeFromDbCallback(" + JSONObject.quote(requestId) + ")")
}
@Suppress("unused")
@JavascriptInterface
fun clearDb(requestId: String) {
(activity.application as App).keyValStore.clearAll(activity)
callJavaScriptFunction(FN_PREFIX + "clearDbCallback('" + requestId + "')")
callJavaScriptFunction(FN_PREFIX + "clearDbCallback(" + JSONObject.quote(requestId) + ")")
}
@Suppress("unused")

View file

@ -15,9 +15,17 @@ ordered so each item is independently shippable and reviewable.
- ✅ `SqliteOpLogAdapter` fully implemented against a minimal `SqliteDb` port,
unit-tested against an in-memory SQLite stand-in. **Not wired to any
platform; no native plugin dependency.**
- ✅ App-private local backup shipped (#7924): `LocalBackupService` writes a
JSON snapshot every 5 min on Android (`KeyValStore` rows `backup` /
`backup_prev`) and iOS (`Directory.Data` `super-productivity-backup.json` /
`.prev.json`), with an empty-state write guard and a two-generation ring so
one bad/evicted write cycle can't erase the only good copy. Fresh-launch
restore prompt is informed (`summarizeBackupStr` shows task / project
counts). Electron continues to use its own rotated backup folder.
Nothing below changes runtime behavior for existing users until step B3 flips a
platform to the SQLite backend.
Nothing in the SQLite tracks below changes runtime behavior for existing users
until step B3 flips a platform to the SQLite backend. The #7924 local-backup
work is already live on Android/iOS.
---
@ -32,28 +40,38 @@ SQLite work. Highest user value per unit effort; do these first.
native and logs nothing when `persist()` resolves `false`. On Android WebView
the grant is often not honored, so today a report like #7892 carries no signal.
- **Do:** `Log.log({ persisted, granted })` on every branch (incl. native), and
surface the result in the exported logs / "About" diagnostics.
- **Do:** `Log.log({ persisted, granted })` on every branch (incl. native), so
exported logs always carry the durability state of the WebView store.
Optionally surface in About diagnostics as a follow-up.
- **Size:** ~1 file, a few lines. **Risk:** none (logging only).
- **Payoff:** the next #7892-style report is conclusive instead of a mystery.
- **Payoff:** the next #7892-style report is conclusive instead of a mystery,
and the telemetry decides whether the next protective steps (e.g. the
near-empty write guard below) are worth the added complexity.
### A2. Periodic local auto-backup outside the WebView store (native)
### A2 (shipped). Debounced on-data-change backup trigger
A second copy of the op-log/state in app-private storage that OS WebView
eviction cannot touch.
✅ Shipped in #7925: `LocalBackupService._triggerBackupSave$` merges a
`LOCAL_ACTIONS`-driven trigger with the existing 5-min interval — any local
action settles into a backup after a 30s quiet period. `LOCAL_ACTIONS`
already filters out remote/hydration replays, and the existing empty-state
guard in `_backup()` prevents writing a degraded post-eviction snapshot
over a good backup, so the trigger strictly adds frequency without spam.
- **Do:** on native, periodically (and on a debounced "data changed") write a
JSON snapshot via `@capacitor/filesystem` to `Directory.Data`; keep the last
N. Restore offered on a wholly-fresh launch when a backup exists. Hook into
the existing `_initBackups()` / `loadStateCache()` flow that already has the
native restore-prompt scaffolding.
- **Size:** medium, isolated feature. **Risk:** low (additive; never deletes the
live store).
- **Payoff:** survives the exact overnight-eviction scenario in #7892 even
before SQLite lands.
### A3 (shipped). Near-empty write-time overwrite guard
> A1 + A2 are the recommended near-term fix for #7892. SQLite (Track B) is the
> durable architectural fix but is weeks of on-device work behind these.
✅ Shipped in #7925: `LocalBackupService._backupAndroid()` and `_backupIOS()`
each read the existing primary slot before promoting/overwriting, and bail
when a near-empty snapshot (< 3 tasks) would clobber a substantial existing
backup (≥ 10 tasks). Counts include active + young-archived + old-archived
tasks via the shared `countAllTasks` helper, so the threshold is the same
on the read side (`summarizeBackupStr`) and the write side. Electron is
unchanged — its rotated, timestamped backup chain isn't a single-slot
overwrite. Fail-safe: skipping never loses data; the guard self-clears
once the store grows back past 3 tasks, so a legitimate bulk-delete is
captured on the next tick.
> A1, A2, and A3 have shipped — Track A is complete. SQLite (Track B) is
> the durable architectural fix and is tracked in #7931.
---
@ -130,9 +148,47 @@ and the `adoptConnection` bridge once SQLite is the sole native backend.
## Suggested order
1. **A1** (trivial, unblocks diagnosis) → **A2** (the real near-term #7892 fix).
2. **B1 → B2 → B3** (gets SQLite runnable + validated behind a flag).
3. **C1 → C2** (migrate real users' data, staged).
4. **D** (tidy up once SQLite is the native default).
1. ✅ Track A complete — **A1** (storage-persistence diagnostics) → **A2**
(debounced data-change trigger) → **A3** (near-empty write-time overwrite
guard) all shipped.
2. **B1 → B2 → B3** (gets SQLite runnable + validated behind a flag) —
tracked in #7931.
3. **C1 → C2** (migrate real users' data, staged) — tracked in #7931.
4. **D** (tidy up once SQLite is the native default) — tracked in #7931.
Tracks A and B/C/D are independent — A can ship while B is still in progress.
Tracks A and B/C/D are independent — A shipped while B/C/D moves at its own
device-gated cadence.
## Cross-cutting / hardening
These don't belong to a single track but were surfaced by the #7924 review and
should land alongside the next time the area is touched.
- **`JavaScriptInterface.kt` JS-literal injection** (Android bridge). The
`loadFromDbCallback(...)` call is built by raw single-quote interpolation of
the stored value into `evaluateJavascript`. Beyond the security smell, it is
a real functional bug: `JSON.stringify` does not escape `'`, so a backup
blob containing an apostrophe terminates the JS string literal and
load-from-DB returns garbage. Fix is to use `JSONObject.quote()` for the
arguments (the same primitive already used by
`emitForegroundServiceStartFailed`).
- **Backup-date in the restore prompt** (strengthens the informed-restore UX
from #7924). iOS has `Filesystem.stat.mtime` for free; Android needs a
bridge change to surface the (now-real) `KEY_CREATED_AT`
`loadFromDbWrapped(key)` returns only the value, so add a meta-aware reader
(e.g. `loadFromDbWithMeta``{ value, createdAt }`). This gives
`KEY_CREATED_AT` its first reader; the column is behaviorally inert today.
- **Robust restore on empty/degraded boot** (was #7901 item 4). Today
`_initBackups()` only offers restore when there is no `stateCache` at all.
Extend the trigger to also fire when the loaded state is degraded per
`hasMeaningfulStateData`. Needs a decision on auto-restore vs prompt and a
guard against resurrecting an intentional wipe (the informed-restore prompt
shipped in #7924 already lets the user decline knowingly).
- **"Last backup" visibility on mobile** (was #7901 item 5). Surface the
most recent successful backup time in About / a settings panel so no-sync
users can see they are protected. Pairs naturally with the backup-date
bridge change.
- **No-sync onboarding nudge** (was #7901 item 6). On a no-sync mobile
install, surface that local-only data is at risk and recommend enabling
sync. Default-on local backup (since #7924) already protects them; this is
the awareness piece.

View file

@ -353,46 +353,48 @@ export class StartupService {
}
private _requestPersistence(): void {
if (navigator.storage) {
// try to avoid data-loss
Promise.all([navigator.storage.persisted()])
.then(([persisted]) => {
if (!persisted) {
return navigator.storage.persist().then((granted) => {
if (granted) {
Log.log('Persistent store granted');
}
// NOTE: we never show this warning for native mobile apps or Electron,
// because persistence is managed by the OS and not subject to browser eviction.
// Also suppress during active onboarding to avoid confusing first-time users.
else if (
!this._platformService.isNative &&
!IS_ELECTRON &&
!OnboardingHintService.isOnboardingInProgress()
) {
const msg = T.GLOBAL_SNACK.PERSISTENCE_DISALLOWED;
Log.warn('Persistence not allowed');
this._snackService.open({ msg });
}
});
} else {
Log.log('Persistence already allowed');
return;
}
})
.catch((e) => {
Log.log(e);
const err = e && e.toString ? e.toString() : 'UNKNOWN';
const msg = T.GLOBAL_SNACK.PERSISTENCE_ERROR;
this._snackService.open({
type: 'ERROR',
msg,
translateParams: {
err,
},
});
});
// A1 (#7925): always log the outcome so a #7892-style report carries the
// durability state of the WebView store. Snack gating below is unchanged.
const isNative = this._platformService.isNative;
if (!navigator.storage) {
Log.log('Persistence: navigator.storage unavailable', { isNative, IS_ELECTRON });
return;
}
navigator.storage
.persisted()
.then((persisted) => {
if (persisted) {
Log.log('Persistence: already granted', { isNative, IS_ELECTRON });
return;
}
return navigator.storage.persist().then((granted) => {
Log.log('Persistence: persist() resolved', {
granted,
isNative,
IS_ELECTRON,
});
// Native + Electron persistence is OS-managed (not subject to browser
// eviction); also suppress during onboarding.
if (
!granted &&
!isNative &&
!IS_ELECTRON &&
!OnboardingHintService.isOnboardingInProgress()
) {
Log.warn('Persistence not allowed');
this._snackService.open({ msg: T.GLOBAL_SNACK.PERSISTENCE_DISALLOWED });
}
});
})
.catch((e) => {
Log.log('Persistence: error', { isNative, IS_ELECTRON, error: e });
const err = e && e.toString ? e.toString() : 'UNKNOWN';
this._snackService.open({
type: 'ERROR',
msg: T.GLOBAL_SNACK.PERSISTENCE_ERROR,
translateParams: { err },
});
});
}
private _checkAvailableStorage(): void {

View file

@ -1,4 +1,6 @@
import {
countAllTasks,
countAllTasksInBackupStr,
isUsableBackupStr,
selectBestBackupStr,
summarizeBackupStr,
@ -96,4 +98,53 @@ describe('backup-ring.util', () => {
expect(selectBestBackupStr('', '')).toBeNull();
});
});
describe('countAllTasks (A3 / #7925)', () => {
it('returns 0 for null / undefined / non-object input', () => {
expect(countAllTasks(null)).toBe(0);
expect(countAllTasks(undefined)).toBe(0);
expect(countAllTasks(42)).toBe(0);
expect(countAllTasks('str')).toBe(0);
});
it('counts active + young-archived + old-archived tasks', () => {
expect(
countAllTasks({
task: { ids: ['t1', 't2'] },
archiveYoung: { task: { ids: ['a1', 'a2'] } },
archiveOld: { task: { ids: ['a3'] } },
}),
).toBe(5);
});
it('treats missing slots as zero rather than throwing', () => {
expect(countAllTasks({})).toBe(0);
expect(countAllTasks({ task: { ids: ['t1'] } })).toBe(1);
});
it('matches summarizeBackupStr.taskCount on a representative blob', () => {
// Single-source-of-truth invariant: both call sites must agree on
// "how many tasks?" so the A3 write-time threshold lines up with
// what summarizeBackupStr shows in the restore prompt.
expect(summarizeBackupStr(WITH_ARCHIVE_AND_INBOX)?.taskCount).toBe(5);
expect(countAllTasksInBackupStr(WITH_ARCHIVE_AND_INBOX)).toBe(5);
});
});
describe('countAllTasksInBackupStr (A3 / #7925)', () => {
it('returns null for empty / corrupt blobs (= no existing backup to compare against)', () => {
expect(countAllTasksInBackupStr(null)).toBeNull();
expect(countAllTasksInBackupStr(undefined)).toBeNull();
expect(countAllTasksInBackupStr('')).toBeNull();
expect(countAllTasksInBackupStr('{broken')).toBeNull();
});
it('returns 0 for a parseable but task-less blob', () => {
expect(countAllTasksInBackupStr(EMPTY_STATE)).toBe(0);
});
it('returns the active+archive sum for a normal backup', () => {
expect(countAllTasksInBackupStr(MEANINGFUL)).toBe(1);
});
});
});

View file

@ -9,6 +9,47 @@ const entityCount = (val: unknown): number => {
const archiveTaskCount = (archive: unknown): number =>
entityCount((archive as { task?: unknown })?.task);
/**
* Counts active + young-archived + old-archived tasks on a backup-shaped
* object. Single source of truth for the "how many tasks?" question used
* by both the informed restore prompt (`summarizeBackupStr`) and the A3
* near-empty write-time overwrite guard (#7925), so "near-empty" means the
* same thing on the read side and the write side.
*/
export const countAllTasks = (data: unknown): number => {
if (!data || typeof data !== 'object') {
return 0;
}
const d = data as {
task?: unknown;
archiveYoung?: unknown;
archiveOld?: unknown;
};
return (
entityCount(d.task) +
archiveTaskCount(d.archiveYoung) +
archiveTaskCount(d.archiveOld)
);
};
/**
* Parse-and-count for the A3 guard: returns null when the stored blob is
* empty/corrupt (treat as "no existing backup", so we don't skip the write
* and lose the chance to capture a real first backup).
*/
export const countAllTasksInBackupStr = (
str: string | null | undefined,
): number | null => {
if (!str) {
return null;
}
try {
return countAllTasks(JSON.parse(str));
} catch {
return null;
}
};
/** A human-meaningful summary of a backup blob, used for the restore prompt. */
export interface BackupSummary {
taskCount: number;
@ -37,10 +78,7 @@ export const summarizeBackupStr = (
const s = JSON.parse(str) as Record<string, unknown>;
const projectIds = (s.project as { ids?: unknown })?.ids;
return {
taskCount:
entityCount(s.task) +
archiveTaskCount(s.archiveYoung) +
archiveTaskCount(s.archiveOld),
taskCount: countAllTasks(s),
projectCount: Array.isArray(projectIds)
? projectIds.filter((id) => id !== INBOX_PROJECT.id).length
: 0,

View file

@ -1,5 +1,5 @@
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
import { BehaviorSubject, of } from 'rxjs';
import { BehaviorSubject, of, Subject } from 'rxjs';
import { LocalBackupService } from './local-backup.service';
import { GlobalConfigService } from '../../features/config/global-config.service';
import { StateSnapshotService } from '../../op-log/backup/state-snapshot.service';
@ -11,8 +11,11 @@ import { initialTimeTrackingState } from '../../features/time-tracking/store/tim
import { CapacitorPlatformService } from '../../core/platform/capacitor-platform.service';
import { AppDataComplete } from '../../op-log/model/model-config';
import { T } from '../../t.const';
import { LOCAL_ACTIONS } from '../../util/local-actions.token';
import { Action } from '@ngrx/store';
const BACKUP_INTERVAL = 5 * 60 * 1000;
const DATA_CHANGE_DEBOUNCE = 30 * 1000;
type LocalBackupServiceWithPrivate = {
_backup: () => Promise<void>;
@ -118,6 +121,8 @@ describe('LocalBackupService', () => {
{ provide: SnackService, useValue: snackServiceSpy },
{ provide: TranslateService, useValue: translateServiceSpy },
{ provide: CapacitorPlatformService, useValue: platformServiceSpy },
// Default is an inert stream; the data-change trigger specs override.
{ provide: LOCAL_ACTIONS, useValue: new Subject<Action>() },
],
});
@ -297,6 +302,7 @@ describe('LocalBackupService', () => {
{ provide: SnackService, useValue: snackServiceSpy },
{ provide: TranslateService, useValue: translateServiceSpy },
{ provide: CapacitorPlatformService, useValue: platformServiceSpy },
{ provide: LOCAL_ACTIONS, useValue: new Subject<Action>() },
],
});
service = TestBed.inject(LocalBackupService);
@ -321,6 +327,209 @@ describe('LocalBackupService', () => {
}));
});
describe('near-empty overwrite guard (A3, #7925)', () => {
type LocalBackupServiceWithA3 = {
_isNearEmptyOverwrite: (
newData: AppDataComplete,
existingRaw: string | null,
) => boolean;
};
type LocalBackupServiceWithIosRing = {
_readIOSFileOrNull: (path: string) => Promise<string | null>;
_writeIOSFile: (path: string, data: string) => Promise<void>;
};
const makeData = (taskCount: number): AppDataComplete =>
({
task: {
ids: Array.from({ length: taskCount }, (_, i) => `t${i}`),
entities: {},
},
archiveYoung: { task: { ids: [], entities: {} } },
archiveOld: { task: { ids: [], entities: {} } },
}) as unknown as AppDataComplete;
const makeStoredBackup = (activeTasks: number, archivedTasks: number = 0): string =>
JSON.stringify({
task: {
ids: Array.from({ length: activeTasks }, (_, i) => `t${i}`),
entities: {},
},
archiveYoung: {
task: {
ids: Array.from({ length: archivedTasks }, (_, i) => `a${i}`),
entities: {},
},
},
archiveOld: { task: { ids: [], entities: {} } },
});
describe('_isNearEmptyOverwrite (pure)', () => {
it('blocks when new < 3 tasks AND existing >= 10 tasks', () => {
const guard = (service as unknown as LocalBackupServiceWithA3)
._isNearEmptyOverwrite;
expect(guard.call(service, makeData(0), makeStoredBackup(10))).toBe(true);
expect(guard.call(service, makeData(2), makeStoredBackup(10))).toBe(true);
expect(guard.call(service, makeData(2), makeStoredBackup(50))).toBe(true);
});
it('counts archived tasks toward the "substantial" threshold', () => {
// 4 active + 8 archived = 12 → still substantial.
const guard = (service as unknown as LocalBackupServiceWithA3)
._isNearEmptyOverwrite;
expect(guard.call(service, makeData(1), makeStoredBackup(4, 8))).toBe(true);
});
it('allows when the new snapshot is not near-empty', () => {
const guard = (service as unknown as LocalBackupServiceWithA3)
._isNearEmptyOverwrite;
expect(guard.call(service, makeData(3), makeStoredBackup(100))).toBe(false);
expect(guard.call(service, makeData(10), makeStoredBackup(10))).toBe(false);
});
it('allows when the existing backup is not substantial', () => {
// A legitimate fresh-start scenario: existing has only a few tasks too.
const guard = (service as unknown as LocalBackupServiceWithA3)
._isNearEmptyOverwrite;
expect(guard.call(service, makeData(1), makeStoredBackup(9))).toBe(false);
expect(guard.call(service, makeData(0), makeStoredBackup(0))).toBe(false);
});
it('allows when there is no existing backup or it is corrupt', () => {
// First-ever write must not be blocked, and a corrupt slot must not
// pretend to be a substantial backup.
const guard = (service as unknown as LocalBackupServiceWithA3)
._isNearEmptyOverwrite;
expect(guard.call(service, makeData(0), null)).toBe(false);
expect(guard.call(service, makeData(0), '')).toBe(false);
expect(guard.call(service, makeData(0), '{broken')).toBe(false);
});
});
describe('integration via _backupIOS', () => {
beforeEach(() => {
(
service as unknown as {
_platformService: Pick<CapacitorPlatformService, 'isIOS'>;
}
)._platformService = { isIOS: () => true };
});
it('skips the overwrite when a near-empty snapshot would clobber a substantial backup', async () => {
stateSnapshotServiceSpy.getAllSyncModelDataFromStoreAsync.and.resolveTo(
makeData(1) as any,
);
spyOn(
service as unknown as LocalBackupServiceWithIosRing,
'_readIOSFileOrNull',
).and.resolveTo(makeStoredBackup(20));
const writeSpy = spyOn(
service as unknown as LocalBackupServiceWithIosRing,
'_writeIOSFile',
).and.resolveTo();
await (service as unknown as LocalBackupServiceWithPrivate)._backup();
// Guard fired before any write — neither the prev promotion nor the
// primary overwrite happened, so the good backup is preserved.
expect(writeSpy).not.toHaveBeenCalled();
});
it('writes normally when the new snapshot is not near-empty', async () => {
stateSnapshotServiceSpy.getAllSyncModelDataFromStoreAsync.and.resolveTo(
makeData(5) as any,
);
spyOn(
service as unknown as LocalBackupServiceWithIosRing,
'_readIOSFileOrNull',
).and.resolveTo(makeStoredBackup(20));
const writeSpy = spyOn(
service as unknown as LocalBackupServiceWithIosRing,
'_writeIOSFile',
).and.resolveTo();
await (service as unknown as LocalBackupServiceWithPrivate)._backup();
// Prev-promotion + primary write = 2 calls.
expect(writeSpy).toHaveBeenCalledTimes(2);
});
});
});
describe('debounced data-change trigger (A2, #7925)', () => {
const setup = (
isEnabled: boolean,
): { actions$: Subject<Action>; backupSpy: jasmine.Spy } => {
const actions$ = new Subject<Action>();
const cfg$ = new BehaviorSubject({ localBackup: { isEnabled } });
globalConfigServiceSpy = jasmine.createSpyObj('GlobalConfigService', [], {
cfg$,
});
TestBed.resetTestingModule();
TestBed.configureTestingModule({
providers: [
LocalBackupService,
{ provide: GlobalConfigService, useValue: globalConfigServiceSpy },
{ provide: StateSnapshotService, useValue: stateSnapshotServiceSpy },
{ provide: BackupService, useValue: backupServiceSpy },
{ provide: SnackService, useValue: snackServiceSpy },
{ provide: TranslateService, useValue: translateServiceSpy },
{ provide: CapacitorPlatformService, useValue: platformServiceSpy },
{ provide: LOCAL_ACTIONS, useValue: actions$ },
],
});
service = TestBed.inject(LocalBackupService);
const backupSpy = spyOn(
service as unknown as LocalBackupServiceWithPrivate,
'_backup',
).and.resolveTo();
service.init();
return { actions$, backupSpy };
};
it('fires one backup after the debounce settles, regardless of action count', fakeAsync(() => {
const { actions$, backupSpy } = setup(true);
actions$.next({ type: 'SomeAction' });
actions$.next({ type: 'AnotherAction' });
actions$.next({ type: 'YetAnother' });
// Inside the debounce window — no backup yet.
tick(DATA_CHANGE_DEBOUNCE - 1);
expect(backupSpy).not.toHaveBeenCalled();
// Window settles — exactly one backup.
tick(1);
expect(backupSpy).toHaveBeenCalledTimes(1);
}));
it('resets the debounce on each new action', fakeAsync(() => {
const { actions$, backupSpy } = setup(true);
actions$.next({ type: 'A' });
tick(DATA_CHANGE_DEBOUNCE - 1);
// New action just before the window closes — debounce resets.
actions$.next({ type: 'B' });
tick(DATA_CHANGE_DEBOUNCE - 1);
expect(backupSpy).not.toHaveBeenCalled();
tick(1);
expect(backupSpy).toHaveBeenCalledTimes(1);
}));
it('does not fire when isEnabled is false', fakeAsync(() => {
const { actions$, backupSpy } = setup(false);
actions$.next({ type: 'SomeAction' });
tick(DATA_CHANGE_DEBOUNCE);
tick(BACKUP_INTERVAL);
expect(backupSpy).not.toHaveBeenCalled();
}));
});
describe('informed mobile restore prompt (#7901)', () => {
type LocalBackupServiceWithPrompt = {
_restoreMobilePromptMsg: (backupData: string) => string;

View file

@ -1,9 +1,10 @@
import { DestroyRef, inject, Injectable } from '@angular/core';
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
import { GlobalConfigService } from '../../features/config/global-config.service';
import { EMPTY, interval, Observable } from 'rxjs';
import { EMPTY, interval, merge, Observable } from 'rxjs';
import { LocalBackupConfig } from '../../features/config/global-config.model';
import { map, switchMap, tap } from 'rxjs/operators';
import { debounceTime, map, switchMap, tap } from 'rxjs/operators';
import { LOCAL_ACTIONS } from '../../util/local-actions.token';
import { LocalBackupMeta } from './local-backup.model';
import { IS_ANDROID_WEB_VIEW } from '../../util/is-android-web-view';
import { IS_ELECTRON } from '../../app.constants';
@ -14,7 +15,12 @@ import { T } from '../../t.const';
import { TranslateService } from '@ngx-translate/core';
import { AppDataComplete } from '../../op-log/model/model-config';
import { hasMeaningfulStateData } from '../../op-log/validation/has-meaningful-state-data.util';
import { selectBestBackupStr, summarizeBackupStr } from './backup-ring.util';
import {
countAllTasks,
countAllTasksInBackupStr,
selectBestBackupStr,
summarizeBackupStr,
} from './backup-ring.util';
import { SnackService } from '../../core/snack/snack.service';
import { Log } from '../../core/log';
import { confirmDialog } from '../../util/native-dialogs';
@ -22,15 +28,19 @@ import { CapacitorPlatformService } from '../../core/platform/capacitor-platform
import { Directory, Encoding, Filesystem } from '@capacitor/filesystem';
const DEFAULT_BACKUP_INTERVAL = 5 * 60 * 1000;
// A2 (#7925): high enough that a flurry of UI actions settles into one backup;
// low enough that a real change is captured before the user backgrounds the app.
const DATA_CHANGE_BACKUP_DEBOUNCE = 30 * 1000;
const ANDROID_DB_KEY = 'backup';
// Previous-generation slot for the two-generation ring (#7901): the current
// `backup` is promoted here before being overwritten, so one bad/corrupt write
// cycle can never erase the only good copy.
// Previous-generation slot for the two-generation ring (#7901).
const ANDROID_DB_KEY_PREV = 'backup_prev';
const IOS_BACKUP_FILENAME = 'super-productivity-backup.json';
const IOS_BACKUP_PREV_FILENAME = 'super-productivity-backup.prev.json';
// const DEFAULT_BACKUP_INTERVAL = 6 * 1000;
// A3 (#7925) near-empty write-time overwrite guard thresholds. Starting point
// — revisit once A1 telemetry shows what a real post-eviction boot looks like.
const NEAR_EMPTY_NEW_TASKS = 3;
const SUBSTANTIAL_EXISTING_TASKS = 10;
@Injectable({
providedIn: 'root',
@ -43,12 +53,24 @@ export class LocalBackupService {
private _snackService = inject(SnackService);
private _translateService = inject(TranslateService);
private _platformService = inject(CapacitorPlatformService);
private _localActions$ = inject(LOCAL_ACTIONS);
private _cfg$: Observable<LocalBackupConfig> = this._configService.cfg$.pipe(
map((cfg) => cfg.localBackup),
);
// A2 (#7925): the empty-state guard in `_backup()` plus A3's near-empty
// guard keep degraded data out — `bulkApplyOperations` / `loadAllData`
// do transit LOCAL_ACTIONS (they aren't tagged `meta.isRemote`), and
// that's fine because the downstream guards handle it.
private _triggerBackupSave$: Observable<unknown> = this._cfg$.pipe(
switchMap((cfg) => (cfg.isEnabled ? interval(DEFAULT_BACKUP_INTERVAL) : EMPTY)),
switchMap((cfg) =>
cfg.isEnabled
? merge(
interval(DEFAULT_BACKUP_INTERVAL),
this._localActions$.pipe(debounceTime(DATA_CHANGE_BACKUP_DEBOUNCE)),
)
: EMPTY,
),
tap(() => this._backup()),
);
@ -81,20 +103,14 @@ export class LocalBackupService {
}
async loadBackupAndroid(): Promise<string> {
// Restore from the newest usable ring slot (#7901). The Android bridge can
// hand back literal newlines, so we escape them here (the single escape site)
// and judge usability on that parse-ready form; the returned string is ready
// for JSON.parse. Returns '' when nothing usable exists (degrades to the
// existing import-error snack rather than throwing on the startup path).
const [primaryRaw, prevRaw] = await Promise.all([
// Restore from the newest usable ring slot (#7901). Returns '' when nothing
// usable exists (degrades to the existing import-error snack rather than
// throwing on the startup path).
const [primary, prev] = await Promise.all([
androidInterface.loadFromDbWrapped(ANDROID_DB_KEY),
androidInterface.loadFromDbWrapped(ANDROID_DB_KEY_PREV),
]);
const best = selectBestBackupStr(
this._escapeAndroidNewlines(primaryRaw),
this._escapeAndroidNewlines(prevRaw),
);
return best ?? '';
return selectBestBackupStr(primary, prev) ?? '';
}
async loadBackupIOS(): Promise<string> {
@ -181,52 +197,73 @@ export class LocalBackupService {
const data =
(await this._stateSnapshotService.getAllSyncModelDataFromStoreAsync()) as AppDataComplete;
// GUARD (#7901/#7892): never overwrite a good on-device backup with an
// empty/degraded store. The local backups live in durable, non-evictable
// storage (Android SQLite KeyValStore, iOS file, Electron file), but after a
// WebView IndexedDB eviction the live NgRx store can boot empty — and the
// 5-min timer would then clobber the last good backup with nothing. Skipping
// the write is always safe: the previous backup stays intact (this mirrors
// the snapshot/compaction empty-overwrite guard). Trade-off: a deliberate
// full wipe is not captured in the local backup until real data exists again.
// #7901/#7892: don't overwrite a good backup with an empty store (a
// post-eviction WebView can boot blank). A3 (#7925) extends this to
// "near-empty over substantial" inside each platform writer.
if (!hasMeaningfulStateData(data)) {
Log.warn(
'LocalBackupService: Skipping backup — current state has no meaningful ' +
'data (refusing to overwrite backup with empty state)',
);
Log.warn('LocalBackupService: skipping backup — empty state');
return;
}
if (IS_ELECTRON) {
// Electron keeps its own rotated, timestamped backups (electron/backup.ts),
// so it needs no ring here.
// Electron has its own rotated, timestamped chain — no ring or A3 guard
// needed (the bug class A3 protects against doesn't apply).
window.ea.backupAppData(data);
}
if (IS_ANDROID_WEB_VIEW) {
await this._backupAndroid(JSON.stringify(data));
await this._backupAndroid(data);
}
if (this._platformService.isIOS()) {
await this._backupIOS(data);
}
}
/**
* Android two-generation ring (#7901): promote the current backup to the prev
* slot before overwriting it, so a single bad write can't erase the only copy.
*/
private async _backupAndroid(dataStr: string): Promise<void> {
// A3 (#7925) pure predicate — kept on its own so the threshold logic is
// testable independent of the platform I/O paths.
private _isNearEmptyOverwrite(
newData: AppDataComplete,
existingRaw: string | null,
): boolean {
if (countAllTasks(newData) >= NEAR_EMPTY_NEW_TASKS) {
return false;
}
const existingTaskCount = countAllTasksInBackupStr(existingRaw);
if (existingTaskCount === null) {
return false;
}
return existingTaskCount >= SUBSTANTIAL_EXISTING_TASKS;
}
// Single source of the A3 skip log; returns true when the caller should bail.
private _guardNearEmptyOverwrite(
data: AppDataComplete,
existing: string | null,
platform: 'Android' | 'iOS',
): boolean {
if (!this._isNearEmptyOverwrite(data, existing)) {
return false;
}
Log.warn(
`LocalBackupService: skipping ${platform} backup — near-empty ` +
`(${countAllTasks(data)} tasks) over substantial backup ` +
`(${countAllTasksInBackupStr(existing)} tasks). #7925 A3.`,
);
return true;
}
private async _backupAndroid(data: AppDataComplete): Promise<void> {
const existing = await androidInterface.loadFromDbWrapped(ANDROID_DB_KEY);
if (this._guardNearEmptyOverwrite(data, existing, 'Android')) return;
if (existing) {
await androidInterface.saveToDbWrapped(ANDROID_DB_KEY_PREV, existing);
}
await androidInterface.saveToDbWrapped(ANDROID_DB_KEY, dataStr);
await androidInterface.saveToDbWrapped(ANDROID_DB_KEY, JSON.stringify(data));
}
private async _backupIOS(data: AppDataComplete): Promise<void> {
try {
// Two-generation ring (#7901): promote the current backup file to the prev
// slot before overwriting, so a single bad write can't erase the only copy.
const existing = await this._readIOSFileOrNull(IOS_BACKUP_FILENAME);
if (this._guardNearEmptyOverwrite(data, existing, 'iOS')) return;
if (existing) {
await this._writeIOSFile(IOS_BACKUP_PREV_FILENAME, existing);
}
@ -246,11 +283,6 @@ export class LocalBackupService {
});
}
/** Re-escapes literal newlines from the Android bridge so the blob parses as JSON. */
private _escapeAndroidNewlines(raw: string | null): string | null {
return raw === null ? null : raw.replace(/\n/g, '\\n');
}
private async _readIOSFileOrNull(path: string): Promise<string | null> {
try {
const result = await Filesystem.readFile({