feat: port client data into the encrypted database at first boot#7408
feat: port client data into the encrypted database at first boot#7408diegolmello wants to merge 2 commits into
Conversation
Add a wipe-and-restore migration that runs once at cold boot, before any server data is read or re-auth is evaluated, while the bootsplash is still up. It ports users, server lock fields, server history, pending messages, drafts, uploads and frequently-used emojis from the legacy plaintext WatermelonDB files into the new SQLCipher database, then deletes the legacy files and their WAL/SHM sidecars. The orchestrator is a crash-safe resumable state machine: each phase transition is recorded in MMKV before the destructive step that follows, so a crash resumes from the last durable phase. A done marker fast-paths every subsequent boot. Wire it into the init saga's restore: open the servers database first so the native key store is installed and the migration ports through the real key, then run the migration. A migration failure is logged and never blocks boot.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
WalkthroughAdds a complete one-shot legacy WatermelonDB-to-encrypted-SQLite migration system with MMKV-backed phase state, platform-aware legacy DB discovery and reading, column-drift-safe row porting helpers, and a phase-driven orchestrator that resumes from persisted state on every boot. The ChangesLegacy DB Migration System
CI Workflow Update
Sequence Diagram(s)sequenceDiagram
participant restore as restore (init.js)
participant orchestrator as runMigrationIfNeeded
participant state as state.ts
participant legacyReader as legacyReader.ts
participant port as port.ts
restore->>orchestrator: runMigrationIfNeeded()
orchestrator->>state: isMigrationDone()
alt already done or skipped
orchestrator-->>restore: return early
else detect: legacy servers DB missing
orchestrator->>state: markSkipped()
orchestrator-->>restore: return
else detect: legacy servers DB present
orchestrator->>state: setPhase(porting_servers)
orchestrator->>legacyReader: openLegacy(servers DB)
orchestrator->>legacyReader: readLegacyUsers / readLegacyServerLockFields / readLegacyServersHistory
orchestrator->>port: portUsers, portServerLockFields, portServersHistory
orchestrator->>state: setPhase(porting_active)
loop each server URL
orchestrator->>legacyReader: openLegacy(per-server DB)
orchestrator->>port: portPendingMessages, portSubscriptionDrafts, portThreadDrafts, portUploads, portFrequentlyUsedEmojis
orchestrator->>state: markServer(url, ported)
end
orchestrator->>state: setPhase(wiping)
loop each server
orchestrator->>orchestrator: secureDelete(per-server DB + WAL/SHM)
orchestrator->>state: markServer(url, wiped)
end
orchestrator->>orchestrator: secureDelete(servers DB)
orchestrator->>state: markDone()
orchestrator-->>restore: return
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/lib/database/migration/__tests__/migration.test.ts (1)
101-140: ⚡ Quick winAdd explicit return types (and typed mock interfaces) for test helpers.
Line 101, Line 118, Line 215, and Line 226 rely on inferred return types. In strict TypeScript suites, this makes mock contract drift easier to miss.
Proposed typed patch
+interface MockSqlite { + runAsync: (sql: string, args?: unknown[]) => Promise<void>; + execAsync: () => Promise<void>; + getFirstAsync: () => Promise<{ count: number }>; + getAllAsync: (sql: string) => Promise<unknown[]>; + closeAsync: () => Promise<void>; +} + -function mockMakeNewSqlite(dbName: string) { +function mockMakeNewSqlite(dbName: string): MockSqlite { if (!mockNewDbWrites[dbName]) mockNewDbWrites[dbName] = []; return { runAsync: jest.fn(async (sql: string, args?: unknown[]) => { mockNewDbWrites[dbName].push({ sql, args: args ?? [] }); }), execAsync: jest.fn(async () => {}), getFirstAsync: jest.fn(async () => ({ count: 0 })), getAllAsync: jest.fn(async (sql: string) => { const tbl = sql.match(/PRAGMA\s+table_info\((\w+)\)/i)?.[1]; if (tbl) return (mockNewDbColumns[tbl] ?? []).map(name => ({ name })); return []; }), closeAsync: jest.fn(async () => {}) }; } -function mockMakeLegacySqlite(dbName: string) { +function mockMakeLegacySqlite(dbName: string): MockSqlite { return { runAsync: jest.fn(async () => {}), execAsync: jest.fn(async () => {}), getFirstAsync: jest.fn(async () => ({ count: 0 })), getAllAsync: jest.fn(async (sql: string) => { const tbl = sql.match(/FROM\s+(\w+)/i)?.[1]; if (!tbl) return []; const all = (mockLegacyRows[dbName]?.[tbl] ?? []) as Record<string, unknown>[]; @@ closeAsync: jest.fn(async () => {}) }; } @@ -function clearAll() { +function clearAll(): void { mockMmkvStore.clear(); @@ } -function seedLegacyDb(dbName: string, table: string, rows: Record<string, unknown>[]) { +function seedLegacyDb(dbName: string, table: string, rows: Record<string, unknown>[]): void { if (!mockLegacyRows[dbName]) mockLegacyRows[dbName] = {}; mockLegacyRows[dbName][table] = rows; }As per coding guidelines, “Use TypeScript for type safety; add explicit type annotations to function parameters and return types” and “Prefer interfaces over type aliases for defining object shapes in TypeScript.”
Also applies to: 215-229
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/database/migration/__tests__/migration.test.ts` around lines 101 - 140, Add explicit return type annotations to the test helper functions mockMakeNewSqlite and mockMakeLegacySqlite (and any other similar functions in the 215-229 range) to prevent type inference issues. First, create typed interfaces that define the shape of the mock database objects being returned, then apply these interfaces as explicit return types to each helper function. This ensures the mock contract is clearly defined and makes any drift in the actual implementation immediately obvious during type checking.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/database/migration/legacyReader.ts`:
- Around line 125-130: The function enumeratePresentServerDbs is returning
filtered server URLs instead of the database names it derives. Modify the
function to return the actual database names that are derived using
deriveServerDbName and validated with legacyFileExists, rather than returning
the original serverUrls array. Use map instead of filter or adjust the filter
approach to capture and return the dbName values that pass the legacyFileExists
check.
In `@app/lib/database/migration/orchestrator.ts`:
- Around line 147-156: The phase is being advanced to 'porting_active' before
the server workset is persisted, creating a crash-window vulnerability. Move the
setPhase('porting_active') call to after the for loop that marks all servers as
pending and after the final readState() call, ensuring the complete servers map
is durable before advancing the phase. The correct order should be: initialize
servers with markServer calls, read the updated state, then call
setPhase('porting_active').
---
Nitpick comments:
In `@app/lib/database/migration/__tests__/migration.test.ts`:
- Around line 101-140: Add explicit return type annotations to the test helper
functions mockMakeNewSqlite and mockMakeLegacySqlite (and any other similar
functions in the 215-229 range) to prevent type inference issues. First, create
typed interfaces that define the shape of the mock database objects being
returned, then apply these interfaces as explicit return types to each helper
function. This ensures the mock contract is clearly defined and makes any drift
in the actual implementation immediately obvious during type checking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2d04edb8-4cbc-480d-b2ee-dfbf4a160789
📒 Files selected for processing (7)
app/lib/database/migration/__tests__/legacyReader.android.test.tsapp/lib/database/migration/__tests__/migration.test.tsapp/lib/database/migration/legacyReader.tsapp/lib/database/migration/orchestrator.tsapp/lib/database/migration/port.tsapp/lib/database/migration/state.tsapp/sagas/init.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/database/migration/__tests__/legacyReader.android.test.tsapp/sagas/init.jsapp/lib/database/migration/state.tsapp/lib/database/migration/orchestrator.tsapp/lib/database/migration/__tests__/migration.test.tsapp/lib/database/migration/port.tsapp/lib/database/migration/legacyReader.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/lib/database/migration/__tests__/legacyReader.android.test.tsapp/lib/database/migration/state.tsapp/lib/database/migration/orchestrator.tsapp/lib/database/migration/__tests__/migration.test.tsapp/lib/database/migration/port.tsapp/lib/database/migration/legacyReader.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/lib/database/migration/__tests__/legacyReader.android.test.tsapp/sagas/init.jsapp/lib/database/migration/state.tsapp/lib/database/migration/orchestrator.tsapp/lib/database/migration/__tests__/migration.test.tsapp/lib/database/migration/port.tsapp/lib/database/migration/legacyReader.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/lib/database/migration/__tests__/legacyReader.android.test.tsapp/sagas/init.jsapp/lib/database/migration/state.tsapp/lib/database/migration/orchestrator.tsapp/lib/database/migration/__tests__/migration.test.tsapp/lib/database/migration/port.tsapp/lib/database/migration/legacyReader.ts
🧠 Learnings (2)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/database/migration/__tests__/legacyReader.android.test.tsapp/lib/database/migration/state.tsapp/lib/database/migration/orchestrator.tsapp/lib/database/migration/__tests__/migration.test.tsapp/lib/database/migration/port.tsapp/lib/database/migration/legacyReader.ts
📚 Learning: 2026-05-07T13:19:52.152Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7304
File: app/sagas/deepLinking.js:237-243
Timestamp: 2026-05-07T13:19:52.152Z
Learning: In this codebase’s Redux-Saga usage, remember that `yield put(action)` dispatches through the Redux store synchronously, and any saga(s) that synchronously react via action listeners (and synchronous `put` chains) will run to completion before the calling saga resumes at its next `yield`. As a result, within a single saga there is no scheduler interleaving between a `yield select(...)` and a subsequent `yield take(...)` at the next `yield` point, so a check-then-take pattern like `const state = yield select(...); if (state !== TARGET) { yield take(a => a.type === TARGET); }` is safe from TOCTOU races under the synchronous `put`/take model described above.
Applied to files:
app/sagas/init.js
🔇 Additional comments (1)
app/lib/database/migration/__tests__/legacyReader.android.test.ts (1)
1-43: LGTM!
| export function enumeratePresentServerDbs(serverUrls: string[]): string[] { | ||
| return serverUrls.filter(url => { | ||
| const dbName = deriveServerDbName(url); | ||
| return legacyFileExists(dbName); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Return content mismatches the function contract
enumeratePresentServerDbs claims to return DB names, but it currently returns filtered server URLs. Any caller using this output for file operations will use the wrong identifiers.
Suggested fix
export function enumeratePresentServerDbs(serverUrls: string[]): string[] {
- return serverUrls.filter(url => {
- const dbName = deriveServerDbName(url);
- return legacyFileExists(dbName);
- });
+ return serverUrls.map(url => deriveServerDbName(url)).filter(dbName => legacyFileExists(dbName));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function enumeratePresentServerDbs(serverUrls: string[]): string[] { | |
| return serverUrls.filter(url => { | |
| const dbName = deriveServerDbName(url); | |
| return legacyFileExists(dbName); | |
| }); | |
| } | |
| export function enumeratePresentServerDbs(serverUrls: string[]): string[] { | |
| return serverUrls.map(url => deriveServerDbName(url)).filter(dbName => legacyFileExists(dbName)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/migration/legacyReader.ts` around lines 125 - 130, The
function enumeratePresentServerDbs is returning filtered server URLs instead of
the database names it derives. Modify the function to return the actual database
names that are derived using deriveServerDbName and validated with
legacyFileExists, rather than returning the original serverUrls array. Use map
instead of filter or adjust the filter approach to capture and return the dbName
values that pass the legacyFileExists check.
| // Record servers as pending in state so porting_active can resume per-server | ||
| setPhase('porting_active'); | ||
| state = readState(); | ||
| // Initialise per-server entries if not already set (fresh run from detect) | ||
| if (state && Object.keys(state.servers).length === 0) { | ||
| for (const url of serverUrls) { | ||
| markServer(url, 'pending'); | ||
| } | ||
| state = readState(); | ||
| } |
There was a problem hiding this comment.
Phase is advanced before server workset is durable
setPhase('porting_active') is called before persisting the full servers map. A crash in this window can resume with empty/partial state.servers, skip active-server porting, and still finish migration as done.
Suggested fix (persist workset first, then flip phase)
// Capture server URLs before closing the legacy DB; we need them for porting_active
const serverUrls = await readLegacyServerUrls(legacyDb);
- // Record servers as pending in state so porting_active can resume per-server
- setPhase('porting_active');
- state = readState();
- // Initialise per-server entries if not already set (fresh run from detect)
- if (state && Object.keys(state.servers).length === 0) {
- for (const url of serverUrls) {
- markServer(url, 'pending');
- }
- state = readState();
- }
+ // Persist per-server workset first; resume stays in porting_servers until this is durable
+ for (const url of serverUrls) {
+ const current = readState();
+ if (!current?.servers[url]) {
+ markServer(url, 'pending');
+ }
+ }
+
+ setPhase('porting_active');
+ state = readState();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/migration/orchestrator.ts` around lines 147 - 156, The phase
is being advanced to 'porting_active' before the server workset is persisted,
creating a crash-window vulnerability. Move the setPhase('porting_active') call
to after the for loop that marks all servers as pending and after the final
readState() call, ensuring the complete servers map is durable before advancing
the phase. The correct order should be: initialize servers with markServer
calls, read the updated state, then call setPhase('porting_active').
The pull_request trigger's branches filter is matched against the base ref. The '*' glob does not match refs containing '/', so PRs based on a feature branch (e.g. feat/native-1277-facade-cutover) never ran the build pipeline. '**' matches across slashes, restoring builds for stacked PRs.
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
requested a deployment
to
android_build
GitHub Actions
Waiting
|
iOS Build Available Rocket.Chat 4.74.0.109131 |
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Review: wipe-and-restore migration
Boot trigger verified fixed 🎉 — initServers now runs as the first yield call inside the takeLatest(APP.INIT, restore) handler, so the previously-observed dropped-APP.INIT problem (root-saga prefix delaying the listener registration) does not occur here; migration fires on every cold boot and the MMKV fast-path keeps subsequent boots O(1).
The ported set is correctly scoped and the wipe ordering is right. Main concern is the non-atomic phase transition (inline) — a crash in that window silently destroys the local-only set it is meant to preserve.
Reviewed by an automated pass; treat inline comments as suggestions, not blockers.
| const serverUrls = await readLegacyServerUrls(legacyDb); | ||
|
|
||
| // Record servers as pending in state so porting_active can resume per-server | ||
| setPhase('porting_active'); |
There was a problem hiding this comment.
🟡 [medium] TOCTOU window between setPhase('porting_active') and markServer calls
setPhase('porting_active') is written first with servers: {}. If the process is killed before any markServer(url, 'pending') call completes, the resumed run enters porting_active with an empty server list, the for-loop body never executes, and the orchestrator falls straight through to wiping — silently destroying all per-server local data (drafts, pending messages, frequently-used emojis).
Fix: write the phase transition and the per-server 'pending' entries in a single atomic state write:
// Replace setPhase + markServer loop with one writeState call:
writeState({
...(readState() ?? { schema: 1, startedAt: getNowMs() }),
phase: 'porting_active',
servers: Object.fromEntries(serverUrls.map(url => [url, 'pending'])),
updatedAt: getNowMs()
});
state = readState();This eliminates the phase=porting_active, servers={} window entirely.
| continue; | ||
| } | ||
|
|
||
| const legacyDb = await openLegacy(dbName); |
There was a problem hiding this comment.
🟢 [low] Uncaught openLegacy throw aborts the entire migration on a corrupt legacy file
openLegacy(dbName) is called before the try block. If it throws (corrupt but present file), the exception propagates out of runMigrationIfNeeded, isMigrationDone() stays false, and every subsequent boot re-enters and re-throws — the migration never completes.
Fix: wrap each server's work in a per-server try/catch and mark it skipped on failure so the loop continues:
let legacyDb;
try {
legacyDb = await openLegacy(dbName);
} catch (e) {
Log.w('[migration] cannot open legacy DB for ' + url + ', skipping', e);
markServer(url, 'ported');
continue;
}| export function readState(): MigrationState | null { | ||
| const raw = userPreferences.getMap(KEY_STATE); | ||
| if (!raw || typeof raw !== 'object') return null; | ||
| return raw as MigrationState; |
There was a problem hiding this comment.
🟢 [low] readState() performs an unchecked cast — corrupt or schema-mismatched state is silently misrouted
return raw as MigrationState trusts the MMKV entry unconditionally. A corrupted value or a state written by a future schema version will be misrouted (e.g. a missing phase field makes state?.phase undefined, causing the orchestrator to run as if in detect again, potentially re-entering a completed migration).
Fix: validate the schema version before returning:
export function readState(): MigrationState | null {
const raw = userPreferences.getMap(KEY_STATE);
if (!raw || typeof raw !== 'object') return null;
// Reject entries from unknown schema versions to avoid misrouting
if ((raw as { schema?: unknown }).schema !== 1) return null;
return raw as MigrationState;
}
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Structural review (NATIVE-1278). The phase state machine is cleanly typed and the boot trigger is correctly wired (moving database.initServers inside restore unblocks takeLatest(APP.INIT)). No file over ~230 lines. Two structural bugs are inline — fix the phase-advance ordering first; it can permanently lose per-server data on a crash. The rest is cleanup.
| const serverUrls = await readLegacyServerUrls(legacyDb); | ||
|
|
||
| // Record servers as pending in state so porting_active can resume per-server | ||
| setPhase('porting_active'); |
There was a problem hiding this comment.
bug: high — phase advances to porting_active before the server map is persisted. setPhase('porting_active') is written here, then the markServer(url, 'pending') loop runs at ~152. A crash in that window means the next boot skips the porting_servers block (phase already advanced), runs porting_active over an empty servers map (zero iterations), then wipes without unlinking per-server legacy files — permanent loss of pending messages/drafts/uploads plus orphaned .db.db files. Populate the server map while still in porting_servers, then advance the phase, so the marker flips atomically with the map already written. The crash-resume test seeds a populated map and doesn't cover the empty-map resume path that exposes this.
| export function readState(): MigrationState | null { | ||
| const raw = userPreferences.getMap(KEY_STATE); | ||
| if (!raw || typeof raw !== 'object') return null; | ||
| return raw as MigrationState; |
There was a problem hiding this comment.
bug: med — readState() casts MMKV output to MigrationState unchecked. A stale/corrupt stored object (unknown phase, wrong schema) passes the cast silently; every state?.phase === ... block then misses and isMigrationDone() stays false → migration spins forever doing nothing. The schema: 1 field implies versioned validation was intended — add an isValidState guard (check schema === 1 and phase ∈ the known set), discard and return null otherwise.
| * Ports draft_message for subscriptions. Uses INSERT OR IGNORE + UPDATE so that | ||
| * only the draft column is touched if the subscription row doesn't exist yet. | ||
| */ | ||
| export async function portSubscriptionDrafts( |
There was a problem hiding this comment.
slop: moderate — portSubscriptionDrafts / portThreadDrafts are identical except for the table name. Both do INSERT OR IGNORE + UPDATE draft_message. Factor into one portDraftColumn(tableName, rows, db) helper.
| * NEW encrypted files with a single `.db` on both platforms. The orchestrator must use this one for | ||
| * every legacy read/exists/wipe, and the connection one only for opening the new DB. | ||
| */ | ||
| export function deriveServerDbName(serverUrl: string): string { |
There was a problem hiding this comment.
slop: moderate — deriveServerDbName here collides with connection.deriveServerDbName. Same name, different results (this one is the legacy double-suffix path; connection's is the new single-.db path). Importing the wrong one is silent and would address the wrong file. Rename this to deriveLegacyServerDbName.
|
|
||
| // Resume from the recorded phase, or start fresh | ||
| let state = readState(); | ||
| const phase = state?.phase ?? 'detect'; |
There was a problem hiding this comment.
slop: moderate — inconsistent phase dispatch. The first block reads a local phase var; every later block reads state?.phase through an optional chain that silently skips a phase if state were ever null. A single switch (state.phase) (or a phase→handler map) over a non-null state makes the control flow explicit and uniform.
| try { | ||
| // expo-file-system v19 exports File from the top-level package (not /next, which was SDK 52 preview) | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const { File } = require('expo-file-system') as { File: new (path: string) => { exists: boolean } }; |
There was a problem hiding this comment.
slop: moderate — require('expo-file-system') on every call. fileExists lazy-requires per upload row; the orchestrator already top-imports File from expo-file-system. Use a normal top-level import to match the codebase.
| * `serverUrls` comes from the legacy servers DB (read by the orchestrator first). | ||
| * The servers DB itself (`default.db`) is NOT included — check separately via legacyFileExists. | ||
| */ | ||
| export function enumeratePresentServerDbs(serverUrls: string[]): string[] { |
There was a problem hiding this comment.
slop: nit — enumeratePresentServerDbs is exported but never called. The orchestrator iterates state.servers directly. Remove it or use it.
| * Lock fields from legacy servers DB `servers` table. | ||
| * Only the fields that are user-authored / device-local are ported; everything else resyncs. | ||
| */ | ||
| export function readLegacyServerLockFields(db: SQLiteDatabase): Promise< |
There was a problem hiding this comment.
slop: nit — the 5-field lock-field shape is inlined twice (return type and the as Promise<{...}[]> cast). Extract a named type.
1 participant
Proposed changes
Adds a wipe-and-restore migration that runs once at cold boot, before any server data is read or re-auth is evaluated, while the bootsplash is still up. It ports the client-owned data — users, server lock fields, server history, pending messages, subscription and thread drafts, uploads (only those whose file still exists), and frequently-used emojis — from the legacy plaintext WatermelonDB files into the new SQLCipher database, then deletes the legacy files and their WAL/SHM sidecars. Everything else is dropped and resynced from the server.
The orchestrator is a crash-safe resumable state machine: each phase transition is recorded in MMKV before the destructive step that follows, so a crash or kill at any step resumes from the last durable phase. A done marker fast-paths every subsequent boot.
Boot wiring lives in the init saga's
restore: the servers database opens first (installing the native key store so the migration ports through the real device key), then the migration runs. A migration failure is logged and never blocks boot or logs the user out.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1278
How to test or reproduce
.db/.db.dbfixtures), cold-boot the app.-wal/-shmsidecars are deleted, the app proceeds past splash, and the session is preserved (no logout).TZ=UTC pnpm test— migration, legacy-reader and facade/driver suites pass.Verified end-to-end on an Android emulator from a fresh build: full detect → port → wipe → done, both legacy fixtures removed, idempotent on relaunch.
Types of changes
Checklist
Further comments
Stacked on
feat/native-1277-facade-cutover(#7407); review/merge that first. Part of the WatermelonDB → encrypted SQLite cutover (NATIVE-1272).Summary by CodeRabbit
New Features
Bug Fixes
Tests