Skip to content

fix(profile): roll back partial import on checksum/migrate failure#28

Merged
InstaZDLL merged 1 commit into
mainfrom
fix/profile-import-cleanup-on-failure
May 16, 2026
Merged

fix(profile): roll back partial import on checksum/migrate failure#28
InstaZDLL merged 1 commit into
mainfrom
fix/profile-import-cleanup-on-failure

Conversation

@InstaZDLL
Copy link
Copy Markdown
Owner

@InstaZDLL InstaZDLL commented May 16, 2026

Summary

Follow-up to #27. The post-extract steps normalise_migration_checksums and db::profile_db::open used a bare ? to propagate errors, which skipped the cleanup that the extract step already performed. A failure there left the freshly inserted profile row and the partial profile directory on disk β€” the user would see a phantom profile in the selector pointing at a half-imported DB.

  • Factor the existing best-effort cleanup into cleanup_partial_profile(state, profile_id).
  • Call it from all three post-insert failure paths (extract, normalise, open) and re-raise the original error so the message the user sees is the real cause, not a cleanup error.
  • pool.close() on the success path is unchanged.

Test plan

  • cargo check --manifest-path src-tauri/Cargo.toml --all-targets
  • Manually corrupt the bundled data.db inside a test archive, attempt import, confirm the profile row + directory are gone afterwards.
  • Manually inject an unknown migration version in the bundled _sqlx_migrations, attempt import, confirm the rejection path also cleans up.

Summary by CodeRabbit

  • Bug Fixes
    • Strengthened profile import reliability with enhanced error recovery. The import process now ensures thorough cleanup of partial profiles when failures occur during extraction, validation, or migration stages. This prevents incomplete profile data and orphaned entries from remaining in the system after unsuccessful import attempts.

Review Change Stack

Bare `?` after step 3 left the new profile row + directory orphaned if
`normalise_migration_checksums` or `profile_db::open` errored. Factor
the existing cleanup into `cleanup_partial_profile` and call it from
all three post-insert failure paths so a failed import always leaves
the profile list in the pre-import state.
@github-actions github-actions Bot added scope: backend Rust/Tauri backend (src-tauri/) type: fix Bug fix size: s 10-50 lines labels May 16, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

πŸ“ Walkthrough

Walkthrough

The PR adds consistent best-effort rollback cleanup during archive profile import. A new cleanup_partial_profile helper removes partial profile directories and deletes corresponding database rows on failures. This helper is now invoked on extraction, normalization, and database-open errors instead of allowing errors to propagate uncleaned.

Changes

Archive import error handling with consistent rollback

Layer / File(s) Summary
cleanup_partial_profile helper function
src-tauri/src/commands/profile_io.rs
New async helper removes the partially created profile directory and deletes the corresponding profile row, with cleanup errors swallowed.
Apply cleanup in import_profile failure paths
src-tauri/src/commands/profile_io.rs
import_profile now calls cleanup_partial_profile on extraction failure, normalise_migration_checksums failure, and profile database open failure before returning errors.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • InstaZDLL/WaveFlow#27: Introduces the normalise_migration_checksums step in import_profile that this PR now adds error recovery for.

Suggested labels

type: fix, size: m, scope: backend

Poem

🐰 A profile import that's clean and prepared,
Now cleans up the mess when things go errored,
With a helper to roll back the half-baked state,
No orphaned rows or directories wait!
hops contentedly

πŸš₯ Pre-merge checks | βœ… 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description is well-structured and comprehensive, covering the problem, solution, and testing approach, but the test plan indicates manual tests are not yet completed. Confirm that manual tests have been executed and the phantom profile cleanup has been verified before merging.
βœ… Passed checks (4 passed)
Check name Status Explanation
Title check βœ… Passed The title directly describes the main change: adding rollback cleanup for partial profile imports on checksum and migration failures, which matches the changeset's core objective.
Docstring Coverage βœ… Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check βœ… Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check βœ… Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
πŸ“ Generate docstrings
  • Create stacked PR
  • Commit on current branch
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/profile-import-cleanup-on-failure

Comment @coderabbitai help to get the list of available commands and usage tips.

@InstaZDLL InstaZDLL self-assigned this May 16, 2026
@coderabbitai coderabbitai Bot added the size: m 50-200 lines label May 16, 2026
@InstaZDLL InstaZDLL merged commit 921678b into main May 16, 2026
13 checks passed
@InstaZDLL InstaZDLL deleted the fix/profile-import-cleanup-on-failure branch May 16, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: backend Rust/Tauri backend (src-tauri/) size: m 50-200 lines size: s 10-50 lines type: fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant