Skip to content

feat: add SQL schema version guard with migration confirmation parity - #4

Merged
GFrancV merged 4 commits into
mainfrom
feat/schema-migration-guard
Jul 21, 2026
Merged

feat: add SQL schema version guard with migration confirmation parity#4
GFrancV merged 4 commits into
mainfrom
feat/schema-migration-guard

Conversation

@GFrancV

@GFrancV GFrancV commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds the missing backward-compatibility guard for the SQL schema version stored in schema_migrations: opening a vault whose schema is newer than what this build knows how to read now fails with a clear error (SCHEMA_VERSION_TOO_NEW) instead of silently misbehaving.
  • Extends the migration confirmation dialog introduced in feat: wire up vault format update confirmation dialog #3 to also cover forward schema migrations (reason: 'schema'), reusing the same optional-backup flow.
  • Extracts a shared confirmMigrationAndBackup helper + mapOpenVaultError in ipc-handlers.ts, used by both the header-version and schema-version gates.
  • Brings vault:open-with-recovery to full behavioral parity with vault:open: previously it had zero version handling at all (no header-version prompt, no schema guard). Both are now identical.
  • Fixes a parity gap found during manual testing: handleRecoveryUnlock in unlock.tsx didn't special-case the MIGRATION_CANCELLED sentinel like the password-unlock flow does, so canceling a migration during recovery-unlock would have leaked the raw sentinel as a visible error.

Test plan

  • Manual test via pnpm dev with a temporary no-op migration_v2 step (bumping MAX_SCHEMA_VERSION 1→2): opening a v1 test vault surfaced the schema-migration dialog through both password unlock and recovery unlock; Cancel and Confirm (with/without backup) all worked correctly.
  • Reverted the temporary migration and reopened the now-v2 vault: correctly rejected with the friendly "created by a newer version" message through both unlock paths, no data touched.
  • Confirmed a normal, fully-migrated vault still opens with zero prompts through both unlock paths (no regression).
  • pnpm validate passes.

@github-actions

Copy link
Copy Markdown

React Doctor found 12 issues in 3 files · 1 error & 11 warnings · score 69 / 100 (Needs work) · vs main

Errors

11 warnings

src/main/db/migrations.ts

  • ⚠️ L60 await inside a loop async-await-in-loop

src/main/vault/vault.ts

  • ⚠️ L828 await inside a loop async-await-in-loop

src/renderer/src/views/unlock.tsx

  • ⚠️ L29 Large component is hard to read and change no-giant-component
  • ⚠️ L29 Many related useState calls prefer-useReducer
  • ⚠️ L74 Side effect inside a state updater function no-side-effect-in-state-updater-function
  • ⚠️ L75 Side effect inside a state updater function no-side-effect-in-state-updater-function
  • ⚠️ L143 Loading flag reset outside finally no-loading-flag-reset-outside-finally
  • ⚠️ L170 Loading flag reset outside finally no-loading-flag-reset-outside-finally
  • ⚠️ L221 Use dvh instead of vh for full height prefer-dvh-over-vh
  • ⚠️ L297 transition: all animates everything no-transition-all
  • ⚠️ L355 transition: all animates everything no-transition-all
⚠️ Warning: .github/workflows/react-doctor.yml is configured incorrectly. See below to fix.

React Doctor compares against main to report only the issues this pull request introduces. This run couldn't complete that comparison (usually a shallow CI checkout with no merge base), so it listed every issue in the changed files, including ones that already existed on main.

Add fetch-depth: 0 to the actions/checkout step in .github/workflows/react-doctor.yml so the checkout includes the history React Doctor needs:

 jobs:
   react-doctor:
     steps:
       - uses: actions/checkout@v5
+        with:
+          fetch-depth: 0

       - uses: millionco/react-doctor@v2

To silence this warning, set silence-missing-baseline-warning: true on the React Doctor action.

Reviewed by React Doctor for commit 219be0e. See inline comments for fixes.

Comment thread src/main/db/migrations.ts
}

for (const step of MIGRATIONS) {
if (step.version > currentVersion) await applyMigration(db, step.version, step.run)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Doctor · react-doctor/async-await-in-loop (warning)

This makes the for…of loop slow because each await runs one after another, so collect the independent calls & run them together with await Promise.all(items.map(...))

Fix → Collect the items, then use await Promise.all(items.map(...)) so independent work runs at the same time

Docs

@@ -169,6 +169,9 @@ export function Unlock(): ReactNode {
)
setLoading(false)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React Doctor · react-doctor/no-loading-flag-reset-outside-finally (warning)

This resets a loading/busy flag only on the success path: if the awaited call rejects the reset never runs and the flag stays stuck truthy (a spinner that never stops, a button disabled forever). Move the reset into a finally block, or mirror it on every catch, so it clears on rejection too.

Fix → A trailing setLoading(false) after an await never runs if the awaited call rejects, so the flag stays stuck truthy; reset it in a finally block (or mirror the reset on every catch) so it clears on both paths.

Docs

@GFrancV
GFrancV merged commit f1fb184 into main Jul 21, 2026
3 checks passed
@GFrancV
GFrancV deleted the feat/schema-migration-guard branch July 21, 2026 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant