-
Notifications
You must be signed in to change notification settings - Fork 0
feat: schema migration engine with per-step validation #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b902398
e4631ef
28f08ae
1371f5d
9e4b338
989e1b3
420e329
e14ab09
51e635e
bf5a86b
a698bde
73e15ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,15 @@ | ||
| import { useState, useCallback } from 'react' | ||
| import { FlowprintEditor, useTheme, useSymbolSearch } from '@ruminaider/flowprint-editor' | ||
| import { useState, useCallback, useMemo, useEffect } from 'react' | ||
| import { | ||
| FlowprintEditor, | ||
| MigrationBanner, | ||
| MigrationModal, | ||
| useTheme, | ||
| useSymbolSearch, | ||
| } from '@ruminaider/flowprint-editor' | ||
| import type { RulesDataMap } from '@ruminaider/flowprint-editor' | ||
| import '@ruminaider/flowprint-editor/styles.css' | ||
| import type { FlowprintDocument } from '@ruminaider/flowprint-schema' | ||
| import { isMajorBump } from '@ruminaider/flowprint-schema' | ||
| import { Header } from './components/Header' | ||
| import { WelcomeScreen } from './components/WelcomeScreen' | ||
| import { NewBlueprintWizard } from './components/NewBlueprintWizard' | ||
|
|
@@ -21,6 +28,7 @@ export function App() { | |
| const [settingsOpen, setSettingsOpen] = useState(false) | ||
| const [error, setError] = useState<string | null>(null) | ||
| const [rulesDataMap, setRulesDataMap] = useState<RulesDataMap>({}) | ||
| const [migrationAccepted, setMigrationAccepted] = useState(false) | ||
|
|
||
| const settingsHook = useSettings() | ||
| const { settings } = settingsHook | ||
|
|
@@ -106,6 +114,35 @@ export function App() { | |
| void settingsHook.updateSettings({ theme: next }) | ||
| }, [settings.theme, settingsHook]) | ||
|
|
||
| // Use the migration result from whichever hook opened the file | ||
| const activeMigrationResult = | ||
| fileManager.migrationResult ?? projectDirectory.migrationResult ?? null | ||
| const dismissMigration = useCallback(() => { | ||
| fileManager.clearMigrationResult() | ||
| projectDirectory.clearMigrationResult() | ||
| }, [fileManager, projectDirectory]) | ||
|
|
||
| const needsMigrationModal = useMemo(() => { | ||
| if (!activeMigrationResult || activeMigrationResult.status !== 'migrated') return false | ||
| const hasRequiredOrNotable = activeMigrationResult.changelog.entries.some( | ||
| (e) => e.required || e.notable, | ||
| ) | ||
| const isMajor = isMajorBump( | ||
| activeMigrationResult.fromVersion, | ||
| activeMigrationResult.toVersion, | ||
| ) | ||
| return hasRequiredOrNotable || isMajor | ||
| }, [activeMigrationResult]) | ||
|
|
||
| const isForcedUpgrade = useMemo(() => { | ||
| if (!activeMigrationResult || activeMigrationResult.status !== 'migrated') return false | ||
| return isMajorBump(activeMigrationResult.fromVersion, activeMigrationResult.toVersion) | ||
| }, [activeMigrationResult]) | ||
|
|
||
| useEffect(() => { | ||
| setMigrationAccepted(false) | ||
| }, [activeMigrationResult]) | ||
|
|
||
| const handleClose = useCallback(() => { | ||
| if (fileManager.dirty) { | ||
| if (!window.confirm('You have unsaved changes. Discard and return to the welcome screen?')) { | ||
|
|
@@ -116,7 +153,8 @@ export function App() { | |
| fileManager.setDirty(false) | ||
| setRulesDataMap({}) | ||
| setError(null) | ||
| }, [fileManager]) | ||
| dismissMigration() | ||
| }, [fileManager, dismissMigration]) | ||
|
|
||
| return ( | ||
| <div | ||
|
|
@@ -199,14 +237,33 @@ export function App() { | |
| setSettingsOpen(true) | ||
| }} | ||
| onClose={handleClose} | ||
| saveDisabled={ | ||
| activeMigrationResult?.status === 'future_version' || | ||
| (needsMigrationModal && !migrationAccepted) | ||
| } | ||
| /> | ||
| {activeMigrationResult && activeMigrationResult.status !== 'current' && ( | ||
| <MigrationBanner result={activeMigrationResult} onDismiss={dismissMigration} /> | ||
| )} | ||
| {needsMigrationModal && activeMigrationResult?.status === 'migrated' && ( | ||
| <MigrationModal | ||
| open={!migrationAccepted} | ||
| changelog={activeMigrationResult.changelog} | ||
|
Comment on lines
+240
to
+251
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. migration.status === 'error' isn't included in saveDisabled/readOnly guards — should we disable Save/edit or require user confirmation for error migrations? Finding type: Want Baz to fix this for you? Activate Fixer Other fix methodsPrompt for AI Agents: Heads up! Your free trial ends in 2 days. |
||
| forced={isForcedUpgrade} | ||
| onAccept={() => setMigrationAccepted(true)} | ||
| /> | ||
| )} | ||
| <div style={{ flex: 1, minHeight: 0 }}> | ||
| <FlowprintEditor | ||
| value={doc} | ||
| onChange={handleChange} | ||
| theme={settings.theme} | ||
| symbolSearch={symbolSearch ?? undefined} | ||
| rulesDataMap={rulesDataMap} | ||
| readOnly={ | ||
| activeMigrationResult?.status === 'future_version' || | ||
| (needsMigrationModal && !migrationAccepted) | ||
| } | ||
| showYamlPreview | ||
| showExportButton | ||
| style={{ width: '100%', height: '100%' }} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import { useState, useRef, useCallback, useMemo } from 'react' | ||
| import { parse } from 'yaml' | ||
| import { validateYaml, serialize } from '@ruminaider/flowprint-schema' | ||
| import type { FlowprintDocument } from '@ruminaider/flowprint-schema' | ||
| import { validate, migrate, serialize } from '@ruminaider/flowprint-schema' | ||
| import type { FlowprintDocument, MigrationResult } from '@ruminaider/flowprint-schema' | ||
|
|
||
| export interface UseFileManagerOptions { | ||
| doc: FlowprintDocument | ||
|
|
@@ -19,6 +19,8 @@ export interface UseFileManagerReturn { | |
| dirty: boolean | ||
| setDirty(dirty: boolean): void | ||
| supportsNativeFS: boolean | ||
| migrationResult: MigrationResult | null | ||
| clearMigrationResult(): void | ||
| } | ||
|
|
||
| const YAML_PICKER_TYPES: FilePickerAcceptType[] = [ | ||
|
|
@@ -40,6 +42,7 @@ export function useFileManager(options: UseFileManagerOptions): UseFileManagerRe | |
| const [filePath, setFilePath] = useState<string | null>(null) | ||
| const [dirty, setDirty] = useState(false) | ||
| const [hasFileHandle, setHasFileHandle] = useState(false) | ||
| const [migrationResult, setMigrationResult] = useState<MigrationResult | null>(null) | ||
| const handleRef = useRef<FileSystemFileHandle | null>(null) | ||
|
|
||
| const supportsNativeFS = useMemo(() => hasNativeFS(), []) | ||
|
|
@@ -62,21 +65,44 @@ export function useFileManager(options: UseFileManagerOptions): UseFileManagerRe | |
| const file = await handle.getFile() | ||
| const text = await file.text() | ||
|
|
||
| const result = validateYaml(text) | ||
| if (!result.valid) { | ||
| const firstError = result.errors[0] | ||
| const msg = firstError | ||
| ? `Invalid flowprint file: ${firstError.message} (at ${firstError.path})` | ||
| : 'Invalid flowprint file' | ||
| throw new Error(msg) | ||
| let parsed: FlowprintDocument | ||
| try { | ||
| parsed = parse(text) as FlowprintDocument | ||
| } catch (err) { | ||
| throw new Error( | ||
| `Failed to parse YAML: ${err instanceof Error ? err.message : String(err)}`, | ||
| ) | ||
| } | ||
|
|
||
| const migration = migrate(parsed) | ||
| setMigrationResult(migration) | ||
|
|
||
|
Comment on lines
+68
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Finding types: Want Baz to fix this for you? Activate Fixer Other fix methodsPrompt for AI Agents: Heads up! Your free trial ends in 2 days. |
||
| // Determine which doc to load | ||
| const docToLoad = | ||
| migration.status === 'migrated' | ||
| ? migration.doc | ||
| : migration.status === 'error' | ||
| ? migration.originalDoc | ||
| : parsed | ||
|
|
||
| // Run structural validation (skip for future_version — tool can't validate future schemas) | ||
| if (migration.status !== 'future_version') { | ||
| const validation = validate(docToLoad) | ||
| if (!validation.valid) { | ||
| const firstError = validation.errors.find((e) => e.severity === 'error') | ||
| if (firstError) { | ||
| throw new Error( | ||
| `Invalid flowprint file: ${firstError.message} (at ${firstError.path})`, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const parsed = parse(text) as FlowprintDocument | ||
| setHandle(handle) | ||
| setFileName(file.name) | ||
| setFilePath(file.name) | ||
| setDirty(false) | ||
| onDocLoaded(parsed, file.name) | ||
| onDocLoaded(docToLoad, file.name) | ||
| }, [onDocLoaded, setHandle]) | ||
|
|
||
| const openFileFallback = useCallback((): Promise<void> => { | ||
|
|
@@ -96,21 +122,44 @@ export function useFileManager(options: UseFileManagerOptions): UseFileManagerRe | |
| void file | ||
| .text() | ||
| .then((text) => { | ||
| const result = validateYaml(text) | ||
| if (!result.valid) { | ||
| const firstError = result.errors[0] | ||
| const msg = firstError | ||
| ? `Invalid flowprint file: ${firstError.message} (at ${firstError.path})` | ||
| : 'Invalid flowprint file' | ||
| throw new Error(msg) | ||
| let parsed: FlowprintDocument | ||
| try { | ||
| parsed = parse(text) as FlowprintDocument | ||
| } catch (err) { | ||
| throw new Error( | ||
| `Failed to parse YAML: ${err instanceof Error ? err.message : String(err)}`, | ||
| ) | ||
| } | ||
|
|
||
| const migration = migrate(parsed) | ||
| setMigrationResult(migration) | ||
|
|
||
| // Determine which doc to load | ||
| const docToLoad = | ||
| migration.status === 'migrated' | ||
| ? migration.doc | ||
| : migration.status === 'error' | ||
| ? migration.originalDoc | ||
| : parsed | ||
|
|
||
| // Run structural validation (skip for future_version — tool can't validate future schemas) | ||
| if (migration.status !== 'future_version') { | ||
| const validation = validate(docToLoad) | ||
| if (!validation.valid) { | ||
| const firstError = validation.errors.find((e) => e.severity === 'error') | ||
| if (firstError) { | ||
| throw new Error( | ||
| `Invalid flowprint file: ${firstError.message} (at ${firstError.path})`, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const parsed = parse(text) as FlowprintDocument | ||
| setHandle(null) | ||
| setFileName(file.name) | ||
| setFilePath(null) | ||
| setDirty(false) | ||
| onDocLoaded(parsed, file.name) | ||
| onDocLoaded(docToLoad, file.name) | ||
| resolve() | ||
| }) | ||
| .catch((err: unknown) => { | ||
|
|
@@ -206,6 +255,10 @@ export function useFileManager(options: UseFileManagerOptions): UseFileManagerRe | |
| } | ||
| }, [writeToHandle, saveFileAs, onError]) | ||
|
|
||
| const clearMigrationResult = useCallback(() => { | ||
| setMigrationResult(null) | ||
| }, []) | ||
|
|
||
| return { | ||
| openFile, | ||
| saveFile, | ||
|
|
@@ -216,5 +269,7 @@ export function useFileManager(options: UseFileManagerOptions): UseFileManagerRe | |
| dirty, | ||
| setDirty, | ||
| supportsNativeFS, | ||
| migrationResult, | ||
| clearMigrationResult, | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
activeMigrationResult keeps showing a file's migration state after switching to a project — should we clear fileManager.migrationResult or dismiss the banner on project switch?
Finding type:
Logical Bugs| Severity: 🔴 HighWant Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Heads up!
Your free trial ends in 2 days.
To keep getting your PRs reviewed by Baz, update your team's subscription