Conversation
Add stateVersion field to VmState interface. On load, legacy state files without a version field are auto-migrated to v1 and re-saved. On save, stateVersion is always stamped to CURRENT_STATE_VERSION (1). This plants the flag for future schema migrations.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThe changes introduce a state versioning mechanism for VM state storage with automatic migration support. A new constant Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/stores/memory.ts (1)
12-15: Versioning stamp looks good, but consider migration parity with FileVmStateStore.The
save()correctly stampsstateVersionbefore cloning and storing. However, unlikeFileVmStateStore.load()(which migrates legacy states withoutstateVersion),MemoryVmStateStore.load()does not perform migration.This is likely acceptable since
MemoryVmStateStoreis primarily used for testing and legacy states wouldn't naturally exist in memory. However, if you ever load raw JSON test fixtures directly into the map, they won't be auto-migrated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/memory.ts` around lines 12 - 15, MemoryVmStateStore.save() currently stamps stateVersion but MemoryVmStateStore.load() lacks the migration logic present in FileVmStateStore.load(); update MemoryVmStateStore.load() to detect legacy VmState objects missing stateVersion and run the same migration/path-upgrade routine used by FileVmStateStore.load() (reuse the migration helper or extraction logic) so in-memory fixtures or raw JSON states get auto-migrated before being returned or used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/stores/memory.ts`:
- Around line 12-15: MemoryVmStateStore.save() currently stamps stateVersion but
MemoryVmStateStore.load() lacks the migration logic present in
FileVmStateStore.load(); update MemoryVmStateStore.load() to detect legacy
VmState objects missing stateVersion and run the same migration/path-upgrade
routine used by FileVmStateStore.load() (reuse the migration helper or
extraction logic) so in-memory fixtures or raw JSON states get auto-migrated
before being returned or used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1c6c13e-4b43-4ef1-a583-a2cd4ca25706
📒 Files selected for processing (5)
.changeset/state-versioning.mdsrc/commands/create/state.tssrc/lib/vm-state.tssrc/stores/memory.tstests/unit/vm-state.test.ts
Summary
stateVersionfield toVmStateinterface (current version: 1)stateVersion: 1from the startChanges
src/lib/vm-state.ts— version field, migration logic in load(), version stamp in save()src/commands/create/state.ts— include stateVersion in buildInitialVmState()src/stores/memory.ts— stamp stateVersion in MemoryVmStateStore.save()Test plan
bun run test— 10 tests passbun run lint— cleanstateVersion: 1Summary by CodeRabbit