* chore(ui): removes dead utilities and op-log leftovers [#8260 - Tier A] * update readme and remove-unused-log-imports.ts * restore benchmark test and make runnable
16 KiB
SQLite Migration — Follow-up Plan
Companion to sqlite-migration.md. That doc holds the
architecture and the per-phase design; this one is the actionable backlog
of what remains after the work on branch claude/issue-7892-root-cause-KY1ED,
ordered so each item is independently shippable and reviewable.
Where we are now
- ✅
OpLogDbAdapter/OpLogTxport + declarativeOP_LOG_DB_SCHEMA. - ✅
IndexedDbOpLogAdapter(faithfulidbbackend) — the live backend. - ✅
OperationLogStoreService+ArchiveStoreServicefully routed through the port (no directthis.db), behind a DI factory token (OP_LOG_DB_ADAPTER_FACTORY), IndexedDB-backed on every platform today. - ✅
SqliteOpLogAdapterfully implemented against a minimalSqliteDbport. Not wired to any platform; no native plugin dependency. - ✅ Validated against a real SQLite engine (sql.js).
sql.jsis served into Karma (dev-only; never in the app bundle) and the adapter's behavioral contract runs against both the in-memory fake and real SQLite (sqlite-op-log-adapter.spec.ts), plus a store-level second pass throughOperationLogStoreService(remote-apply-store-port.integration.spec.ts). Confirms the realUNIQUE constraint failed→ConstraintErrormapping,AUTOINCREMENT-after-clear(), compound-index + NULL ranges, and realBEGIN IMMEDIATErollback. (B2 translation-layer + store-port passes; the on-device real-engine run still remains.) - ✅ C1 backend migration implemented + tested (
op-log-backend-migration.ts): whole-DB copy from any source adapter to any dest adapter in one dest transaction with verify-before-commit (op count + lastseq+ vector clock; mismatch → rollback). Validated real-Chrome-IDB → sql.js. Not wired into startup — B3/C2 decide when to run it. - ✅ Backend-aware store init (B3, partial).
OperationLogStoreService.init()/ArchiveStoreService._init()now calladapter.init()and skip the IDB open for self-managing backends (noadoptConnection); the IndexedDB path is unchanged. Dead in production until the native token flip. Only the device-gated token override + nativeSqliteDbwrapper remain. - ✅ App-private local backup shipped (#7924):
LocalBackupServicewrites a JSON snapshot every 5 min on Android (KeyValStorerowsbackup/backup_prev) and iOS (Directory.Datasuper-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 (summarizeBackupStrshows task / project counts). Electron continues to use its own rotated backup folder.
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.
Track A — Ship the #7892 safeguards now (independent of SQLite)
These directly reduce the data-loss blast radius and do not depend on the SQLite work. Highest user value per unit effort; do these first.
A1. Make navigator.storage.persist() observable on native
startup.service._requestPersistence() suppresses the not-granted branch on
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), 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, and the telemetry decides whether the next protective steps (e.g. the near-empty write guard below) are worth the added complexity.
A2 (shipped). Debounced on-data-change backup trigger
✅ 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.
A3 (shipped). Near-empty write-time overwrite guard
✅ 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.
Track B — Finish the SQLite backend (native)
B1. Add @capacitor-community/sqlite + a SqliteDb wrapper
- Do: add the plugin (+ iOS/Android native config), and a ~20-line adapter
from its
SQLiteDBConnectionto theSqliteDbport (run/query). Open one DB namedSUP_OPSinDirectory.Data. - Gotcha: the plugin's web build is WASM-SQLite persisted into
IndexedDB — i.e. it reintroduces the eviction risk. Bind SQLite only when
IS_NATIVE_PLATFORM; web/PWA/Electron stay on IndexedDB. - Size: small. Risk: native build/CI surface.
- ⚡ Perf — bake two mitigations into the wrapper from the start. On native
the dominant cost is the Capacitor JS↔native bridge round-trip, not SQLite
itself. Reads (
getAll/count) are already one query = one crossing. Single-op append is negligible. The one cliff is bulk write:OperationLogStoreService.appendBatch()loopsawait tx.add()once per op, so N ops = N crossings. Mitigations (only matter on the bridge; can't be measured with in-process sql.js, so validate on-device):- Return
lastIdfrom the plugin's ownrunresponse (it provides it) — never issue a separateSELECT last_insert_rowid(), which would double every insert to two crossings. - Add an optional bulk path to the port (e.g.
runBatch(statements)onSqliteDb+ anaddBatchonOpLogTx) soappendBatchcollapses to one crossing via the plugin'sexecuteSet. Per-opseqrecovery from a batched insert needsRETURNING seq(SQLite ≥ 3.35) orlast_insert_rowid()arithmetic over the consecutive AUTOINCREMENT range — pick after confirming the plugin's SQLite version on-device.
- Return
B2. Validate SqliteOpLogAdapter against a real engine — ✅ done (CI), on-device remains
- ✅ sql.js in Karma.
sql.jsis a devDependency, served into Karma as a global script + a proxied.wasm(src/karma.conf.js), so the webpacknode:import problem is sidestepped (loaded as a script, not bundled). A ~25-lineSqlJsDbwrapper (sql-js-db.test-helper.ts) satisfies theSqliteDbport. - ✅ Adapter contract dual-run.
sqlite-op-log-adapter.spec.tsruns the behavioral contract against both the fake and real sql.js; SQL-emission specs stay fake-only. Confirmed the realUNIQUE constraint failed→ConstraintErrormapping,AUTOINCREMENT-after-clear(), compound-index + NULL ranges, realBEGIN IMMEDIATErollback. No surprises surfaced. - ✅ Store-port second pass.
remote-apply-store-port.integration.spec.tsruns the store's composed flows (apply/mark/merge-clock, partial failures, full-state clearing) throughOperationLogStoreServiceon both backends. - ⏳ Remains: the on-device real-engine run. sql.js validates the engine, not
the Capacitor bridge or the native plugin's specific SQLite build/flags. The
operation-log-stress.benchmark.tsharness is the lever for the on-device perf + behavior pass (see B1 perf note). Run it explicitly withnpm run test:file src/app/op-log/testing/benchmarks/operation-log-stress.benchmark.ts; it is compiled bysrc/tsconfig.spec.jsonbut excluded from the auto-run**/*.spec.tsset, so it never runs in normal CI.
B3. Flip the DI token on native — init fix ✅ landed; token flip device-gated
- ✅ Backend-aware store init (the half found during B2 stage 2, now done).
OperationLogStoreService.init()andArchiveStoreService._init()were IDB-shaped — they unconditionally opened+adopted a WebView IndexedDB connection and never calledadapter.init(), so on SQLite the tables were never created and the evictable store was still touched. Now: when the adapter exposes noadoptConnection(self-managing, e.g. SQLite),init()callsawait this._adapter.init()and skips the IDB open; the IndexedDB path is unchanged. Two unit tests cover both branches; the store-port integration spec now drives the store fully on SQLite (no pre-init workaround). The new branch is dead in production until the token flip below, so it shipped risk-free. - ⏳ Remains (device-gated): override
OP_LOG_DB_ADAPTER_FACTORYto returnSqliteOpLogAdapterwhenIS_NATIVE_PLATFORM, behind a feature flag defaulting off. The factory must hand both services' adapters the sameSqliteDb(one SQLite file, all tables) — mirroring how they share one IDB connection today. Needs B1 (the nativeSqliteDbwrapper) first. - ✅ Shared-connection safety (prerequisite for the flip, landed). With both
services on one
SqliteDb, concurrent op-log operations (capture append vs. archive write vs. compaction) would otherwise interleaveBEGINs on the one connection — SQLite has no nested transactions, so a secondBEGINthrows and a bare statement issued mid-transaction joins (and rolls back with) the foreign transaction.SqliteOpLogAdapternow funnels every entry point through a FIFO queue keyed to the shared connection (WeakMap<SqliteDb, …>, not the adapter instance) — so the op-log store's and archive store's separate adapters over the oneSqliteDbserialize against each other, and a transaction is exclusive on the connection for its wholeBEGIN…COMMIT. The port contract documents the invariant; contract tests cover concurrent transactions on both engines and the two-adapters-one-connection topology. (Closes the H-6/#8746 rollout blocker.) Residual: the re-entrancy precondition (atransaction()callback must use itstxhandle, never re-enter an adapter method, or it deadlocks on the queue) is documented but unenforced — a lint rule is the right future guard (a runtime flag can't tell a re-entrant call from a legal concurrent one). - Size: tiny token flip (init + serialization done). Risk: gated by the flag.
Track C — Data migration (native, one-time)
C1. IDB → SQLite copy on first launch after enabling B3 — ✅ algorithm done + tested, wiring remains
- ✅ Algorithm:
migrateOpLogBackend(source, dest)inop-log-backend-migration.tscopies all stores in onedesttransaction with verify-before-commit (op count + lastseq+ vector clock; mismatch → rollback, source untouched). Genericiterate→put: preserves opsseq(incl. gaps) via put-honors-seq, writes singletons at their out-of-line key, no per-store special-casing. Adapter-agnostic, so validated real-Chrome-IDB → sql.js in CI; the native plugin dest behaves identically through the port. - ⏳ Remains (wiring, with B3): detect "SQLite empty/absent and legacy
SUP_OPSpresent" on first launch and callmigrateOpLogBackend. Set a migration-complete marker. Keep the IDB copy untouched for ≥1 release as a fallback. Note: the copy uses the adapter port's read side, independent of the store-init IDB-open fix in B3. - Risk: high (data movement) — mitigated by the verify-before-commit safety net (now tested to actually roll back) + retain-source.
C2. Staged rollout
Beta/dogfood on real Android devices → staged enable → remove the IDB fallback
and the adoptConnection bridge once SQLite is the sole native backend.
Track D — Cleanup (after SQLite is the native default)
- D1. Remove the transitional
adoptConnectionbridge from the port and the two services once no backend relies on a borrowed connection. - D2. Consider deriving the IndexedDB upgrade from
OP_LOG_DB_SCHEMAsorunDbUpgradeonly carries deltas (the schema spec already guards against drift; this removes the remaining hand-maintained duplication). - D3. Out-of-scope for #7892, optional: migrate the other small IDB
databases (
SUPThemes,sup-sync,sup-plugin-oauth) only if fully evacuating WebView storage is desired.
Suggested order
- ✅ Track A complete — A1 (storage-persistence diagnostics) → A2 (debounced data-change trigger) → A3 (near-empty write-time overwrite guard) all shipped.
- B1 → B2 → B3 (gets SQLite runnable + validated behind a flag) — tracked in #7931.
- C1 → C2 (migrate real users' data, staged) — tracked in #7931.
- D (tidy up once SQLite is the native default) — tracked in #7931.
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.ktJS-literal injection (Android bridge). TheloadFromDbCallback(...)call is built by raw single-quote interpolation of the stored value intoevaluateJavascript. Beyond the security smell, it is a real functional bug:JSON.stringifydoes not escape', so a backup blob containing an apostrophe terminates the JS string literal and load-from-DB returns garbage. Fix is to useJSONObject.quote()for the arguments (the same primitive already used byemitForegroundServiceStartFailed).- Backup-date in the restore prompt (strengthens the informed-restore UX
from #7924). iOS has
Filesystem.stat.mtimefor 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 givesKEY_CREATED_ATits 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 nostateCacheat all. Extend the trigger to also fire when the loaded state is degraded perhasMeaningfulStateData. 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.