Custom sqlite migrator to be cloud-sync-ready#5048
Conversation
Signed-off-by: Yujong Lee <yujonglee.dev@gmail.com>
Signed-off-by: Yujong Lee <yujonglee.dev@gmail.com>
Signed-off-by: Yujong Lee <yujonglee.dev@gmail.com>
Signed-off-by: Yujong Lee <yujonglee.dev@gmail.com>
Signed-off-by: Yujong Lee <yujonglee.dev@gmail.com>
✅ Deploy Preview for char-cli-web canceled.
|
✅ Deploy Preview for hyprnote canceled.
|
✅ Deploy Preview for unsigned-char ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6aaa426. Configure here.
| { | ||
| hypr_db_core2::recreate_storage(&options.db)?; | ||
| try_open_db(options, schema).await | ||
| } |
There was a problem hiding this comment.
Recreate policy catches all errors, not just migrations
Medium Severity
The Err(_error) wildcard in open_db catches every AppDbOpenError variant when Recreate policy is active, not just migration failures. The previous open_with_migrate in db-core2 specifically matched only DbOpenError::Migration. Now errors like StepChecksumMismatch, Open (connection/IO failures), InvalidCloudsyncStep, or transient Sqlx errors would all trigger recreate_storage, silently wiping the database. This is a data-loss risk for any future caller that opts into the Recreate policy.
Reviewed by Cursor Bugbot for commit 6aaa426. Configure here.
| let pool = db.pool().as_ref().clone(); | ||
|
|
||
| (dir, pool, DbRuntime::new(std::sync::Arc::new(db))) | ||
| } |
There was a problem hiding this comment.
Tests reference tables missing from new migrations
High Severity
setup_runtime() uses hypr_db_app::schema() which only creates templates, calendars, and events tables. However, nearly every test in runtime.rs (both the integration test and the unit test inside src/runtime.rs) references daily_notes and daily_summaries tables, which no longer exist in the migration steps. All those tests will fail at runtime with "no such table" errors.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 6aaa426. Configure here.
1 participant
No description provided.