fix(editor): treat a malformed legacyEditor envelope as absent, not as a crash - #442
Conversation
…s a crash
`legacyEditorSchema` is `z.object({}).passthrough()`, so zod validates nothing
inside it: a project file whose `speedRegions` is a string loads clean and only
detonates on the first clip edit, with `regions.filter is not a function`. Every
clip delete / move / duplicate / source-range edit routes through
`mapAllRegionCollections`, so the editor is stuck until the file is repaired by
hand.
Guard the two envelope fields the function walks exactly as
`upgradeV4DocumentToV5` already guards them. A non-array is not a collection we
can walk, but it is also not ours to delete: treating it as absent leaves it in
place through the `...legacy` spread, so the rest of the document edits normally
and nothing the user had is thrown away on the way past it.
Tightening the schema so these arrays are validated at load time is the deeper
fix; this is the local one the migration path already models.
Fixes #356
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe timeline mapping path now skips malformed ChangesLegacy editor collection handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change preserves malformed envelope data while preventing editor crashes during clip edits; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
legacyEditorSchemaisz.object({}).passthrough(), so zod validates nothing inside it. A project file whosespeedRegionsis a string loads clean and only detonates on the first clip edit, withregions.filter is not a function. Every clip delete / move / duplicate / source-range edit routes throughmapAllRegionCollections, so the editor is stuck until the file is repaired by hand.The fix
Guard the two envelope fields the function walks exactly as
upgradeV4DocumentToV5already guards them — the migration path has known better all along, the runtime path just never caught up.Preserved, not dropped. A non-array is not a collection we can walk, but it is also not ours to delete: treating it as absent leaves it in place through the
...legacyspread, so the rest of the document edits normally and nothing the user had is thrown away on the way past it. When neither field is walkable the function returnsdocument.legacyEditorby reference, so the envelope comes out bit-identical.Tests
Three cases in
timeline.test.ts, all confirmed to fail on the pre-fix code (TypeError: regions.flatMap is not a function): a malformed field alongside a well-formed neighbour, both fields malformed (asserting the same reference comes back, which pins "preserved" rather than "rebuilt to look the same"), and a second route into the same function viasetClipSourceRange.Scope
Tightening
legacyEditorSchemaso these arrays are validated at load time is the deeper fix and is deliberately not in here. Worth noting for whoever picks that up:anchoredRegionsOfmakes the same unchecked cast on the same two fields. It cannot crash —.flat()does not flatten a non-array andhasCompleteClipAnchorrejects a string — but it is the same latent assumption.Fixes #356
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests