fix(profile): normalise sqlx migration checksums on import + pin sources to LF#27
Merged
Merged
Conversation
…ces to LF `.waveflow` archives produced by builds whose migration files were checked out with CRLF (Windows `core.autocrlf=true`, no `.gitattributes`) embedded SHA-384 checksums in `_sqlx_migrations.checksum` that differ from the same SQL re-hashed today on an LF checkout. Re-importing such an archive crashed with "migration X was previously applied but has been modified" even though the DDL is byte-identical at the semantic level. - `normalise_migration_checksums` rewrites each row in the imported `_sqlx_migrations` to the local migrator's checksum before `sqlx::migrate!` runs. Versions present in the archive but unknown locally are rejected (archive newer than app). - `.gitattributes` pins source files (sql/rs/ts/tsx/js/json/toml/md/...) to LF so future archives stay byte-stable across contributors.
github-actions
Bot
added
scope: backend
📝 WalkthroughWalkthroughThis PR fixes profile import failures caused by line-ending mismatches in SQLx migration checksums. It enforces LF via ChangesMigration Checksum Normalization for Cross-Platform Archives
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/commands/profile_io.rs (1)
226-234:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRollback profile artifacts when post-extract finalization fails.
Line [226] and Line [233] can return early via
?, but this path does not clean up the newly insertedprofilerow/directory like the extract-failure path does. A rejected archive (e.g., unknown migration version) can leave an orphan profile entry and partial files.Suggested fix
- normalise_migration_checksums(&state.paths.profile_db(new_profile_id)).await?; - - // 5. Open + close the imported pool once so any pending migrations - // (the source might be older than the local schema) replay - // immediately. This matches the create_profile flow and gives - // the user a usable profile by the time the call returns. - let pool = - db::profile_db::open(&state.paths.profile_db(new_profile_id), &state.paths.app_db).await?; - pool.close().await; + let finalize_result = async { + normalise_migration_checksums(&state.paths.profile_db(new_profile_id)).await?; + + // 5. Open + close the imported pool once so any pending migrations + // (the source might be older than the local schema) replay + // immediately. This matches the create_profile flow and gives + // the user a usable profile by the time the call returns. + let pool = db::profile_db::open( + &state.paths.profile_db(new_profile_id), + &state.paths.app_db, + ) + .await?; + pool.close().await; + Ok::<(), AppError>(()) + } + .await; + + if let Err(err) = finalize_result { + let _ = std::fs::remove_dir_all(state.paths.profile_dir(new_profile_id)); + let _ = sqlx::query("DELETE FROM profile WHERE id = ?") + .bind(new_profile_id) + .execute(&state.app_db) + .await; + return Err(err); + }🤖 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 `@src-tauri/src/commands/profile_io.rs` around lines 226 - 234, The post-extract finalization steps (normalise_migration_checksums and db::profile_db::open) can early-return with ? and currently won't undo the newly created profile row/directory for new_profile_id; ensure any error from normalise_migration_checksums(&state.paths.profile_db(new_profile_id)) or db::profile_db::open(&state.paths.profile_db(new_profile_id), &state.paths.app_db) triggers cleanup: delete the inserted profile record and remove the profile directory for new_profile_id before returning the error. Implement this by replacing the bare ? with error-handling that calls a cleanup helper (e.g., cleanup_profile_artifacts(state, new_profile_id) or inline logic to remove the DB row and fs::remove_dir_all on state.paths.profile_db(new_profile_id)) and then re-returns the error, and reuse this cleanup on both failing calls; keep pool.close() after successful open as-is.
🤖 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.
Outside diff comments:
In `@src-tauri/src/commands/profile_io.rs`:
- Around line 226-234: The post-extract finalization steps
(normalise_migration_checksums and db::profile_db::open) can early-return with ?
and currently won't undo the newly created profile row/directory for
new_profile_id; ensure any error from
normalise_migration_checksums(&state.paths.profile_db(new_profile_id)) or
db::profile_db::open(&state.paths.profile_db(new_profile_id),
&state.paths.app_db) triggers cleanup: delete the inserted profile record and
remove the profile directory for new_profile_id before returning the error.
Implement this by replacing the bare ? with error-handling that calls a cleanup
helper (e.g., cleanup_profile_artifacts(state, new_profile_id) or inline logic
to remove the DB row and fs::remove_dir_all on
state.paths.profile_db(new_profile_id)) and then re-returns the error, and reuse
this cleanup on both failing calls; keep pool.close() after successful open
as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8da8371d-5c4d-4af7-b704-0e8c039d60a6
📒 Files selected for processing (3)
.gitattributesdocs/features/ui.mdsrc-tauri/src/commands/profile_io.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.waveflowarchives produced by older builds (when migration.sqlfiles were checked out with CRLF endings under Windowscore.autocrlf=trueand no.gitattributes) stored SHA-384 checksums in_sqlx_migrations.checksumcomputed on CRLF bytes. The current source tree has those same migrations checked out as LF, sosqlx::migrate!(compile-time macro that embeds the file bytes) ends up with LF-based checksums. Re-importing an old archive then trips sqlx's "migration X was previously applied but has been modified" guard even though the SQL is semantically identical — making every pre-existing.waveflowexport unimportable on a current build.normalise_migration_checksumsstep inimport_profilerewrites each row of the bundled_sqlx_migrationsto the local migrator's checksum before sqlx's verification runs. Versions present in the archive but unknown to the local migrator (archive produced by a newer WaveFlow than the importing build) are rejected explicitly rather than silently dropped..gitattributespins source files (*.sql,*.rs,*.ts,*.tsx,*.js,*.json,*.toml,*.yml,*.md, …) to LF so future archives stay byte-stable across contributors and the same drift cannot reoccur. Docs updated indocs/features/ui.md.Root-cause evidence
Comparing the
_sqlx_migrations.checksumrows inside a real exported.waveflowagainstSHA-384(file_bytes)of the current source:20260411120000_initial20260413000100_drop_local_metadata_caches20260428000000_drop_metadata_fks20260428000001_track_rating20260428000002_track_audio_quality20260428000003_track_musical_key20260509000000_playlist_cover_hash20260509100000_playlist_cover_is_auto20260510221047_add_spotify_auth_provider20260512230000_perf_indexesMixed CRLF/LF reflects the working-tree state at each build's time; the SQL content itself has only ever been touched by its introducing commit.
Test plan
cargo check --manifest-path src-tauri/Cargo.toml --all-targetsNayeon.waveflow): 7 checksums normalised, the 2 pending migrations (album_artist+invalidate_mtime_for_album_artist_backfill) apply cleanly, 996 tracks + 5 playlists recovered, newalbum.album_artistcolumn present.bun run tauri devbuild.Summary by CodeRabbit
Bug Fixes
Documentation