feat: move SQLite to filesystem migration from TypeScript to Rust #3415
Conversation
- Add new migration v1_0_4_migrate_sqlite_to_filesystem.rs that reads from SQLite and writes to filesystem format during app startup - Remove SQLite loading logic from TypeScript local persister - Make sources module public in importer plugin for reuse - Migration is idempotent and only runs if db.sqlite exists and no session data is present in filesystem This makes the migration atomic and safe from force-quit issues during first launch, as the migration now happens in Rust before TypeScript loads. Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
✅ Deploy Preview for hyprnote-storybook canceled.
|
✅ Deploy Preview for hyprnote canceled.
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| pub fn run(base_dir: &Path) -> Result<()> { | ||
| let sqlite_file = base_dir.join("db.sqlite"); | ||
| if !sqlite_file.exists() { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let sessions_dir = base_dir.join("sessions"); | ||
| if sessions_dir.exists() && has_session_data(&sessions_dir) { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
Critical: Incomplete migration handling leads to data loss
The idempotency check has a race condition that can cause incomplete migrations. If the migration is interrupted after creating some sessions (lines 194-241), the next run will:
- Find
db.sqliteexists (line 18) ✓ - Find
sessions_direxists with_meta.jsonfiles (line 23) ✓ - Skip the entire migration (line 24)
- Leave unmigrated sessions in SQLite permanently
The issue is that has_session_data() returns true if ANY session has a _meta.json, but the per-session skip at line 197-199 only prevents re-creating existing sessions. Interrupted migrations leave some data in SQLite and some on filesystem, with no way to complete the migration.
Fix: The migration should track completion state explicitly:
pub fn run(base_dir: &Path) -> Result<()> {
let sqlite_file = base_dir.join("db.sqlite");
if !sqlite_file.exists() {
return Ok(());
}
// Check for migration completion marker instead
let migration_complete = base_dir.join(".migration_v1_0_4_complete");
if migration_complete.exists() {
return Ok(());
}
let runtime = tokio::runtime::Runtime::new().map_err(|e| crate::Error::Io(e.into()))?;
runtime.block_on(async { migrate_sqlite_to_filesystem(base_dir, &sqlite_file).await })?;
// Mark migration as complete
std::fs::write(&migration_complete, "")?;
Ok(())
}| pub fn run(base_dir: &Path) -> Result<()> { | |
| let sqlite_file = base_dir.join("db.sqlite"); | |
| if !sqlite_file.exists() { | |
| return Ok(()); | |
| } | |
| let sessions_dir = base_dir.join("sessions"); | |
| if sessions_dir.exists() && has_session_data(&sessions_dir) { | |
| return Ok(()); | |
| } | |
| pub fn run(base_dir: &Path) -> Result<()> { | |
| let sqlite_file = base_dir.join("db.sqlite"); | |
| if !sqlite_file.exists() { | |
| return Ok(()); | |
| } | |
| // Check for migration completion marker instead | |
| let migration_complete = base_dir.join(".migration_v1_0_4_complete"); | |
| if migration_complete.exists() { | |
| return Ok(()); | |
| } | |
| let runtime = tokio::runtime::Runtime::new().map_err(|e| crate::Error::Io(e.into()))?; | |
| runtime.block_on(async { migrate_sqlite_to_filesystem(base_dir, &sqlite_file).await })?; | |
| // Mark migration as complete | |
| std::fs::write(&migration_complete, "")?; | |
| Ok(()) | |
| } | |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| fn escape_yaml_string(s: &str) -> String { | ||
| s.replace('\\', "\\\\").replace('"', "\\\"") | ||
| } |
There was a problem hiding this comment.
🔴 YAML escape function doesn't handle newlines, causing malformed frontmatter
The escape_yaml_string function only escapes backslashes and double quotes, but doesn't handle newline characters (\n), carriage returns (\r), or other control characters.
Click to expand
How it triggers
When migrating data from SQLite to filesystem, if any field like title, name, content, or event_id contains a newline character, the generated YAML frontmatter will be malformed:
fn escape_yaml_string(s: &str) -> String {
s.replace('\\', "\\\\").replace('"', "\\\"")
}For example, if a session title is "Meeting\nNotes", it will produce:
title: "Meeting
Notes"Instead of the valid:
title: "Meeting\nNotes"Impact
The malformed YAML will cause parsing failures when the app tries to read the migrated data files (_meta.json, human markdown files, organization markdown files), potentially causing data to be inaccessible or the app to crash on load.
Recommendation: Extend the escape function to handle newlines and other control characters:
fn escape_yaml_string(s: &str) -> String {
s.replace('\\', "\\\\")
.replace('"', "\\\"")
.replace('\n', "\\n")
.replace('\r', "\\r")
.replace('\t', "\\t")
}
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
| let sessions_dir = base_dir.join("sessions"); | ||
| if sessions_dir.exists() && has_session_data(&sessions_dir) { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
🔴 Incomplete migration on interruption due to flawed idempotency check
The migration's idempotency check can skip migrating remaining sessions if the app crashes mid-migration.
Click to expand
Mechanism
The migration at lines 22-25 checks if ANY session has _meta.json before running:
let sessions_dir = base_dir.join("sessions");
if sessions_dir.exists() && has_session_data(&sessions_dir) {
return Ok(());
}But has_session_data() returns true if ANY single session folder contains _meta.json.
Scenario
- Migration starts with sessions A, B, C in SQLite
- Session A is fully written to filesystem (
_meta.jsoncreated) - App crashes/force-quit before sessions B and C are migrated
- On restart,
has_session_data()returnstrue(A has_meta.json) - Entire migration is skipped
- Sessions B and C remain only in SQLite and are never migrated
Impact
Data loss - sessions that weren't migrated before the crash will never be migrated to the filesystem format, even though SQLite still contains them.
Expected behavior
The migration should check per-session whether migration is needed, or track which sessions have been successfully migrated.
Recommendation: Instead of checking for ANY session data, compare the sessions in SQLite against those already migrated to filesystem. Or remove the early return and rely on the per-session if session_dir.exists() { continue; } check at line 197-199, which already provides per-session idempotency.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
1 participant
Summary
This PR moves the SQLite data migration logic from TypeScript (local persister) to Rust (fs-db plugin) as an explicit migration step. Previously, the migration happened in TypeScript during app load, which was vulnerable to data loss if the user force-quit during first launch. Now the migration runs atomically in Rust before TypeScript loads.
Key changes:
v1_0_4_migrate_sqlite_to_filesystem.rsthat reads from SQLite using the existing v1_sqlite importer and writes to the filesystem formatmigrateWordsAndHintsToTranscriptsfunctionsourcesmodule public in importer plugin to allow reuse ofimport_all_from_pathThe migration is idempotent: it only runs if
db.sqliteexists AND no session data is present in the filesystem.Review & Testing Checklist for Human
_meta.json,transcript.json,_memo.md, and enhanced note files match the format expected by the session persister_meta.jsonin sessions dir, but partial writes could leave data in an inconsistent statejson_to_yamlfunction may not handle all edge cases (nested objects, special characters in strings)Recommended test plan:
db.sqliteNotes
LocalPersisterLoadedflag and associated Rust commands are now unused but not removed (can be cleaned up in a follow-up PR)Link to Devin run: https://app.devin.ai/sessions/8a49ec2d87984040997331253f3a3973
Requested by: @yujonglee