feat: add schema migration system for store files by don-petry · Pull Request #19 · oneirosoft/dagger

don-petry · 2026-03-31T02:19:34Z

Why?

As dgr evolves, the on-disk store format (state.json, config.json, operation.json) will need to change. Without a migration system, users upgrading dgr would face cryptic deserialization errors or need to manually recreate their state. A sequential migration runner ensures smooth upgrades and clear error messages when the tool version is too old.

Summary

Adds a sequential migration runner (src/core/store/migrate.rs) that checks version fields on load and applies migrations in order for state.json, config.json, and operation.json
Each loader (load_state, load_config, load_operation) now deserializes to serde_json::Value first, runs the migration function, then deserializes to the typed struct
Future/unknown versions produce a clear "upgrade dgr" error; missing version fields produce a descriptive error
Version parsing uses checked try_into() conversion to prevent silent truncation of large values

Addresses item 6 from #10 (schema migration).

How it works

When a store file is loaded:

Raw JSON is parsed into an untyped serde_json::Value
The migrate_* function checks the version field
If the version is current, the value passes through unchanged
If older, sequential migrations are applied (none yet needed at v1)
If newer than supported, an error tells the user to upgrade dgr
The migrated value is then deserialized into the typed struct

Adding a future migration (e.g., v1 to v2) is a one-line addition:

if version < 2 { value = migrate_state_v1_to_v2(value)?; }

Test plan

Unit tests: current version passes through unchanged (state, config, operation)
Unit tests: future version returns "upgrade dgr" error
Unit tests: missing version field returns descriptive error
Unit tests: invalid version type and u64 overflow handled
CI: cargo test --locked passes

🤖 Generated with Claude Code

Add a sequential migration runner that checks version fields on load and applies migrations in order for state.json, config.json, and operation.json. This ensures future version bumps have a clean upgrade path instead of failing to deserialize old files. - New migrate.rs module with migrate_state, migrate_config, and migrate_operation functions - Each loader now deserializes to serde_json::Value first, runs migration, then deserializes to the typed struct - Future versions produce a clear "upgrade dgr" error - Missing version fields produce a descriptive error - Comprehensive test coverage for all migration paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds a JSON schema migration layer to the store-file loading path so existing on-disk state.json, config.json, and operation.json can be forward-migrated (or rejected when too new) before deserializing into typed Rust structs.

Changes:

Introduces src/core/store/migrate.rs with migrate_state, migrate_config, and migrate_operation functions plus unit tests.
Updates load_state, load_config, and load_operation to deserialize into serde_json::Value, run migrations, then deserialize into typed structs.
Registers the new migrate module in src/core/store/mod.rs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/core/store/migrate.rs	Adds version checking and migration entry points for state/config/operation JSON values with unit tests.
src/core/store/state.rs	Routes state loading through the migration layer before typed deserialization.
src/core/store/config.rs	Routes config loading through the migration layer before typed deserialization.
src/core/store/operation.rs	Routes operation loading through the migration layer before typed deserialization.
src/core/store/mod.rs	Exposes the new `migrate` module within `store`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T02:22:37Z

src/core/store/migrate.rs

+    let version = value
+        .get("version")
+        .and_then(|v| v.as_u64())
+        .ok_or_else(|| {
+            io::Error::new(
+                io::ErrorKind::InvalidData,
+                "state file missing 'version' field",
+            )
+        })? as u32;


version is parsed as u64 and then cast with as u32. If the JSON contains a very large integer (e.g. > u32::MAX), this will truncate and may incorrectly bypass the version > DAGGER_STATE_VERSION check. Also, the error message says "missing 'version' field" even when the field exists but is the wrong type (e.g. string/float/negative). Consider using a checked conversion (try_into) and returning an error like "missing or invalid 'version' field" (and/or a specific "version out of range" error).

Addressed in the latest push — version parsing now uses checked conversion via try_into() with specific error messages for missing, wrong-type, and overflow cases.

Copilot · 2026-03-31T02:22:38Z

src/core/store/migrate.rs

+    let version = value
+        .get("version")
+        .and_then(|v| v.as_u64())
+        .ok_or_else(|| {
+            io::Error::new(
+                io::ErrorKind::InvalidData,
+                "config file missing 'version' field",
+            )
+        })? as u32;


version is parsed as u64 and then cast with as u32, which can truncate large values and lead to incorrect version comparisons (e.g. a huge version might appear "older" after truncation). Also, the current error message treats an invalid type as "missing". Use a checked conversion (try_into) and a clearer "missing or invalid" message for the version field.

Addressed in the latest push — version parsing now uses checked conversion via try_into() with specific error messages for missing, wrong-type, and overflow cases.

Copilot · 2026-03-31T02:22:38Z

src/core/store/migrate.rs

+    let version = value
+        .get("version")
+        .and_then(|v| v.as_u64())
+        .ok_or_else(|| {
+            io::Error::new(
+                io::ErrorKind::InvalidData,
+                "operation file missing 'version' field",
+            )
+        })? as u32;


version is converted via as u32 after reading as u64; this will silently truncate values > u32::MAX, which can cause the future-version guard to be bypassed. Also, a present-but-non-integer version currently yields a "missing" message. Prefer checked conversion and a message indicating the field is missing or invalid/out-of-range.

Addressed in the latest push — version parsing now uses checked conversion via try_into() with specific error messages for missing, wrong-type, and overflow cases.

Split version field parsing into two steps: first check for field presence, then validate the value with try_into() instead of bare `as u32` cast. This catches wrong-type and overflow cases with specific error messages. Addresses Copilot review comments on PR oneirosoft#19. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

don-petry · 2026-03-31T02:30:26Z

All three Copilot review comments addressed in the latest push (de1d418) — version parsing now uses checked conversion via try_into() with specific error messages for missing, wrong-type, and overflow cases. Also added tests for invalid version type and u64 overflow.

Copilot AI review requested due to automatic review settings March 31, 2026 02:19

Copilot started reviewing on behalf of don-petry March 31, 2026 02:20 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

don-petry mentioned this pull request Mar 31, 2026

Correctness & data safety improvements #10

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add schema migration system for store files#19

feat: add schema migration system for store files#19
don-petry wants to merge 2 commits intooneirosoft:mainfrom
don-petry:feat/schema-migration

don-petry commented Mar 31, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

don-petry Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

don-petry Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

don-petry Mar 31, 2026

Uh oh!

don-petry commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

don-petry commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why?

Summary

How it works

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

don-petry Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

don-petry Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

don-petry Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

don-petry commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

don-petry commented Mar 31, 2026 •

edited

Loading