diff --git a/frontend/src/lib/userDraftDbMigration.ts b/frontend/src/lib/userDraftDbMigration.ts index 824cd1c952..4c3079ea94 100644 --- a/frontend/src/lib/userDraftDbMigration.ts +++ b/frontend/src/lib/userDraftDbMigration.ts @@ -1,11 +1,12 @@ /** * One-off migration from the localStorage UserDraft autosave to the - * DB-backed `draft` table. Runs after `migrateLegacyUserDrafts` (which - * produces the `userdraft/w/{workspace}/{kind}/{path}` keys this reads), - * POSTing each to `/drafts/update` and clearing the source key only on - * success — so it's idempotent without a sentinel; failed entries retry next - * mount. Not workspace-gated: keys embed their own workspace and the token - * covers all of them, so gating would orphan other-workspace entries. + * DB-backed `draft` table. Reads the workspace-scoped + * `userdraft/w/{workspace}/{kind}/{path}` keys (written by the editor during + * the interim LS-backed phase, so the embedded workspace is correct), POSTing + * each to `/drafts/update` and clearing the source key only on success — so + * it's idempotent without a sentinel; failed entries retry next mount. Not + * workspace-gated: keys embed their own workspace and the token covers all of + * them, so gating would orphan other-workspace entries. * * Before uploading, each draft is compared against its deployed version * (script / flow / app); a draft that's deep-equal to what's deployed carries diff --git a/frontend/src/lib/userDraftLegacyMigration.test.ts b/frontend/src/lib/userDraftLegacyMigration.test.ts index 7069929897..f7963e55a0 100644 --- a/frontend/src/lib/userDraftLegacyMigration.test.ts +++ b/frontend/src/lib/userDraftLegacyMigration.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach } from 'vitest' import { - migrateLegacyUserDrafts, + purgeLegacyUserDrafts, __resetUserDraftLegacyMigrationForTesting } from './userDraftLegacyMigration' @@ -8,18 +8,12 @@ function encodeLegacy(value: unknown): string { return btoa(encodeURIComponent(JSON.stringify(value))) } -function wrapped(value: V): string { - return JSON.stringify({ value }) -} - -// Read a migrated entry, strip the GC `lastWrittenAt` stamp so assertions -// can match the `{ value }` shape regardless of when the migration ran. -function storedShape(key: string): string | null { - const raw = localStorage.getItem(key) - if (raw == null) return null - const parsed = JSON.parse(raw) - delete parsed.lastWrittenAt - return JSON.stringify(parsed) +const legacyApp = { + grid: [], + fullscreen: false, + theme: undefined, + unusedInlineScripts: [], + hiddenInlineScripts: [] } beforeEach(() => { @@ -27,197 +21,110 @@ beforeEach(() => { __resetUserDraftLegacyMigrationForTesting() }) -describe('migrateLegacyUserDrafts', () => { - it('migrates a legacy app draft to the workspace-scoped key with a { value } wrapper', () => { - // Shape mirrors what the legacy AppEditor wrote: `encodeState($appStore)`, - // i.e. the inner App value, not the wrapping AppWithLastVersion. - const legacyApp = { - grid: [], - fullscreen: false, - theme: undefined, - unusedInlineScripts: [], - hiddenInlineScripts: [] - } +describe('purgeLegacyUserDrafts', () => { + it('drops a recognised legacy app draft without re-creating it under any key', () => { localStorage.setItem('app-u/me/dashboard', encodeLegacy(legacyApp)) - migrateLegacyUserDrafts('main') + purgeLegacyUserDrafts() expect(localStorage.getItem('app-u/me/dashboard')).toBeNull() - expect(storedShape('userdraft/w/main/app/u/me/dashboard')).toBe(wrapped(legacyApp)) + // The workspace-blind key is gone, NOT promoted to a guessed workspace. + expect(localStorage.getItem('userdraft/w/main/app/u/me/dashboard')).toBeNull() }) - it('migrates a legacy empty-path app draft (the `app` literal key)', () => { - const legacyApp = { - grid: [], - fullscreen: false, - unusedInlineScripts: [], - hiddenInlineScripts: [] - } + it('drops the empty-path legacy keys (`app` / `flow` / `rawapp` literals)', () => { localStorage.setItem('app', encodeLegacy(legacyApp)) + localStorage.setItem('flow', encodeLegacy({ flow: { summary: '', value: { modules: [] } } })) + localStorage.setItem('rawapp', encodeLegacy({ files: {}, runnables: {}, data: {} })) - migrateLegacyUserDrafts('main') + purgeLegacyUserDrafts() expect(localStorage.getItem('app')).toBeNull() - expect(storedShape('userdraft/w/main/app/')).toBe(wrapped(legacyApp)) + expect(localStorage.getItem('flow')).toBeNull() + expect(localStorage.getItem('rawapp')).toBeNull() }) - it('migrates a legacy flow draft and strips the view-state envelope', () => { - const flow = { summary: 'f', value: { modules: [] }, path: 'u/me/myflow' } - const legacyBundle = { - flow, - path: 'u/me/myflow', - selectedId: 'settings', - draft_triggers: [{ id: 't1' }], - selected_trigger: null, - loadedFromHistory: undefined - } - localStorage.setItem('flow-u/me/myflow', encodeLegacy(legacyBundle)) + it('drops recognised legacy flow and raw-app drafts', () => { + localStorage.setItem( + 'flow-u/me/myflow', + encodeLegacy({ flow: { summary: 'f', value: { modules: [] } }, selectedId: 'settings' }) + ) + localStorage.setItem( + 'rawapp-u/me/site', + encodeLegacy({ files: { 'index.tsx': 'x' }, runnables: {}, data: {} }) + ) - migrateLegacyUserDrafts('main') + purgeLegacyUserDrafts() expect(localStorage.getItem('flow-u/me/myflow')).toBeNull() - // Only the inner Flow survives; the view-state envelope is dropped. - expect(storedShape('userdraft/w/main/flow/u/me/myflow')).toBe(wrapped(flow)) - }) - - it('migrates a legacy raw-app draft, defaulting the new `summary` field', () => { - const legacy = { - files: { 'index.tsx': 'export default () => null' }, - runnables: {}, - data: { tables: [] } - } - localStorage.setItem('rawapp-u/me/site', encodeLegacy(legacy)) - - migrateLegacyUserDrafts('main') - expect(localStorage.getItem('rawapp-u/me/site')).toBeNull() - expect(storedShape('userdraft/w/main/raw_app/u/me/site')).toBe( - wrapped({ ...legacy, summary: '' }) - ) }) - it('preserves an existing new-format entry instead of overwriting it', () => { - // Old and new both exist for the same item — the new one is presumed - // fresher. - localStorage.setItem( - 'app-u/me/dash', - encodeLegacy({ - grid: [], - fullscreen: false, - unusedInlineScripts: [], - hiddenInlineScripts: [] - }) - ) - const existingNew = wrapped({ value: 'new' }) - localStorage.setItem('userdraft/w/main/app/u/me/dash', existingNew) + it('leaves the workspace-scoped interim keys untouched (migrateUserDraftsToDb owns those)', () => { + const interim = JSON.stringify({ value: { modules: [] } }) + localStorage.setItem('userdraft/w/main/flow/u/me/keep', interim) - migrateLegacyUserDrafts('main') + purgeLegacyUserDrafts() - expect(localStorage.getItem('app-u/me/dash')).toBeNull() - expect(localStorage.getItem('userdraft/w/main/app/u/me/dash')).toBe(existingNew) - }) - - it('is idempotent — the second invocation is a no-op', () => { - localStorage.setItem( - 'app-u/me/dash', - encodeLegacy({ - grid: [], - fullscreen: false, - unusedInlineScripts: [], - hiddenInlineScripts: [] - }) - ) - migrateLegacyUserDrafts('main') - expect(localStorage.getItem('userdraft/w/main/app/u/me/dash')).not.toBeNull() - - // Drop the migrated entry to detect any re-migration attempt. - localStorage.removeItem('userdraft/w/main/app/u/me/dash') - // Drop the source too, so re-running couldn't even find a source. - // (The sentinel alone should be enough; this just clarifies the intent.) - migrateLegacyUserDrafts('main') - expect(localStorage.getItem('userdraft/w/main/app/u/me/dash')).toBeNull() - }) - - it('skips entirely when no workspace is available', () => { - localStorage.setItem( - 'app-u/me/dash', - encodeLegacy({ - grid: [], - fullscreen: false, - unusedInlineScripts: [], - hiddenInlineScripts: [] - }) - ) - migrateLegacyUserDrafts('') - - expect(localStorage.getItem('app-u/me/dash')).not.toBeNull() - }) - - it('handles malformed legacy payloads without throwing', () => { - localStorage.setItem('app-u/me/garbled', 'not-base64!!!') - expect(() => migrateLegacyUserDrafts('main')).not.toThrow() - // Migration didn't migrate, didn't crash — leaves the entry alone. - expect(localStorage.getItem('app-u/me/garbled')).toBe('not-base64!!!') + expect(localStorage.getItem('userdraft/w/main/flow/u/me/keep')).toBe(interim) }) it('leaves keys whose path does not match the legacy `u|f/owner/name` shape alone', () => { - // A future feature or neighbouring code might pick a key like - // `app-recent` for its own purposes. The path doesn't look like a - // Windmill item path, so the migration must skip it. + // `app-recent` / `app-some_other_app` look like the legacy prefix but the + // suffix isn't a Windmill item path — a future feature might own them. localStorage.setItem('app-recent', 'whatever') localStorage.setItem('app-some_other_app', 'whatever') - // `flow-u/me/foo` matches the shape and would be migrated, but the - // payload also needs to look like a Windmill draft (asserted below). - localStorage.setItem('flow-u/me/foo', encodeLegacy({ flow: { value: { modules: [] } } })) - migrateLegacyUserDrafts('main') + purgeLegacyUserDrafts() expect(localStorage.getItem('app-recent')).toBe('whatever') expect(localStorage.getItem('app-some_other_app')).toBe('whatever') - expect(localStorage.getItem('userdraft/w/main/flow/u/me/foo')).not.toBeNull() }) - it('skips legacy-shaped keys whose payload does not look like a Windmill draft', () => { - // `app-u/me/dash` matches LEGACY_PATH_SHAPE and decodes to valid JSON, - // but none of the App-shape fields (grid/fullscreen/theme/ - // unusedInlineScripts/hiddenInlineScripts) are present. Treat it as - // unrelated and leave it untouched. - const unrelated = encodeLegacy({ random: 'data', count: 7 }) - localStorage.setItem('app-u/me/dash', unrelated) + it('leaves legacy-shaped keys whose payload does not look like a Windmill draft', () => { + // Matches LEGACY_PATH_SHAPE and decodes to valid JSON, but carries none of + // the App/flow draft fields — treat as unrelated, do not delete. + const unrelatedApp = encodeLegacy({ random: 'data', count: 7 }) + localStorage.setItem('app-u/me/dash', unrelatedApp) const unrelatedFlow = encodeLegacy({ stepsState: {} }) localStorage.setItem('flow-u/me/bar', unrelatedFlow) - migrateLegacyUserDrafts('main') + purgeLegacyUserDrafts() - expect(localStorage.getItem('app-u/me/dash')).toBe(unrelated) - expect(localStorage.getItem('userdraft/w/main/app/u/me/dash')).toBeNull() + expect(localStorage.getItem('app-u/me/dash')).toBe(unrelatedApp) expect(localStorage.getItem('flow-u/me/bar')).toBe(unrelatedFlow) - expect(localStorage.getItem('userdraft/w/main/flow/u/me/bar')).toBeNull() }) - it('migrates multiple legacy entries in a single invocation', () => { - localStorage.setItem( - 'app-u/me/a', - encodeLegacy({ - grid: [], - fullscreen: false, - unusedInlineScripts: [], - hiddenInlineScripts: [] - }) - ) + it('leaves a malformed (non-base64) legacy payload alone and does not throw', () => { + localStorage.setItem('app-u/me/garbled', 'not-base64!!!') + + expect(() => purgeLegacyUserDrafts()).not.toThrow() + expect(localStorage.getItem('app-u/me/garbled')).toBe('not-base64!!!') + }) + + it('is idempotent — once the sentinel is set, a later legacy key survives', () => { + localStorage.setItem('app-u/me/a', encodeLegacy(legacyApp)) + purgeLegacyUserDrafts() + expect(localStorage.getItem('app-u/me/a')).toBeNull() + + // A key written after the first run is NOT swept (the sentinel short-circuits). + localStorage.setItem('app-u/me/b', encodeLegacy(legacyApp)) + purgeLegacyUserDrafts() + expect(localStorage.getItem('app-u/me/b')).not.toBeNull() + }) + + it('purges multiple legacy entries in a single invocation', () => { + localStorage.setItem('app-u/me/a', encodeLegacy(legacyApp)) localStorage.setItem( 'flow-u/me/b', - encodeLegacy({ flow: { summary: '', value: { modules: [] }, path: 'u/me/b' } }) - ) - localStorage.setItem( - 'rawapp-u/me/c', - encodeLegacy({ files: {}, runnables: {}, data: { tables: [] } }) + encodeLegacy({ flow: { summary: '', value: { modules: [] } } }) ) + localStorage.setItem('rawapp-u/me/c', encodeLegacy({ files: {}, runnables: {}, data: {} })) - migrateLegacyUserDrafts('main') + purgeLegacyUserDrafts() - expect(localStorage.getItem('userdraft/w/main/app/u/me/a')).not.toBeNull() - expect(localStorage.getItem('userdraft/w/main/flow/u/me/b')).not.toBeNull() - expect(localStorage.getItem('userdraft/w/main/raw_app/u/me/c')).not.toBeNull() + expect(localStorage.getItem('app-u/me/a')).toBeNull() + expect(localStorage.getItem('flow-u/me/b')).toBeNull() + expect(localStorage.getItem('rawapp-u/me/c')).toBeNull() }) }) diff --git a/frontend/src/lib/userDraftLegacyMigration.ts b/frontend/src/lib/userDraftLegacyMigration.ts index d4ee04ee61..4239bc0e8a 100644 --- a/frontend/src/lib/userDraftLegacyMigration.ts +++ b/frontend/src/lib/userDraftLegacyMigration.ts @@ -1,22 +1,29 @@ /** - * One-off migration from the pre-UserDraft localStorage autosave entries to - * the workspace-scoped `userdraft/w/{ws}/{kind}/{path}` format. + * One-shot purge of the pre-UserDraft browser-local autosave keys. * - * Legacy keys (global, not workspace-scoped — assumed to belong to the user's - * current workspace at migration time): + * The original autosave (pre-#9121) wrote workspace-BLIND keys: * - * `flow` / `flow-{path}` base64 of `encodeState({ flow, path, selectedId, draft_triggers, ... })` - * `app` / `app-{path}` base64 of `encodeState(App)` - * `rawapp` / `rawapp-{path}` base64 of `encodeState({ files, runnables, data })` + * `flow` / `flow-{path}` base64 of `encodeState({ flow, path, selectedId, draft_triggers, ... })` + * `app` / `app-{path}` base64 of `encodeState(App)` + * `rawapp` / `rawapp-{path}` base64 of `encodeState({ files, runnables, data })` * - * Target keys: `userdraft/w/{workspace}/{flow|app|raw_app}/{path}` storing - * `JSON.stringify({ value: })`. + * Neither the key nor the decoded value records a workspace (the value carries + * only workspace-agnostic item paths like `u/me/x`), so these drafts cannot be + * attributed to the workspace they were edited in. The current editors are + * DB-backed and never read these keys, so they are dead data with one dangerous + * property: promoting them to the DB would force a GUESS of the workspace, which + * mis-files drafts into whatever workspace happened to be active when the + * migration first ran (a single global sentinel gates it). We therefore drop + * them instead of migrating them. + * + * Only keys that BOTH match the legacy path shape AND decode to a plausible + * legacy draft are removed; unrelated look-alikes (`app-recent`, garbage, + * non-Windmill payloads) are left untouched. The workspace-scoped interim keys + * (`userdraft/w/{ws}/...`, written by the editor with the correct workspace) + * are NOT touched here — `migrateUserDraftsToDb` still pushes those to the DB. * * Idempotent: writes a sentinel under `MIGRATION_FLAG` after the first run so - * subsequent invocations are no-ops. Existing new-format entries are never - * overwritten — when both an old and a new entry exist for the same item, the - * old one is simply dropped on the assumption that the new entry is the more - * recent edit. + * subsequent invocations are no-ops. * * This file is intentionally standalone — it does not import from * `userDraft.svelte.ts` so the new code stays uncluttered by the legacy @@ -71,10 +78,9 @@ function decodeLegacyState(raw: string): unknown { * Per-kind shape gate. The legacy keys (`app-foo`, `flow-foo`, ...) are * unusual enough that nothing else in the codebase has used them, but * matching `LEGACY_PATH_SHAPE` doesn't prove the payload is actually a - * Windmill draft (any base64-of-JSON could pass). Promoting a stray payload - * would silently surface as a phantom "Restored from local storage" toast - * on the next edit, so we reject anything that doesn't carry the fields the - * legacy writers actually produced. + * Windmill draft (any base64-of-JSON could pass). We only delete keys we can + * positively recognise as legacy drafts, so a stray look-alike that happens to + * use this key shape is left untouched rather than silently dropped. */ function isPlausibleLegacyValue(kind: LegacyKind, decoded: unknown): boolean { if (decoded == null || typeof decoded !== 'object') return false @@ -103,32 +109,6 @@ function isPlausibleLegacyValue(kind: LegacyKind, decoded: unknown): boolean { } } -function transformLegacyValue(kind: LegacyKind, decoded: unknown): unknown { - const obj = decoded as Record - switch (kind) { - case 'flow': - // The legacy bundle wrapped the Flow alongside view-state fields - // (selectedId, draft_triggers, ...). The new entry stores only the - // Flow — the view-state lives elsewhere or is re-derived. - return obj.flow - case 'app': - // Legacy stored the App directly. - return obj - case 'raw_app': - // Legacy bundle missed the `summary` field that the new editor adds. - return { - files: obj.files ?? {}, - runnables: obj.runnables ?? {}, - data: obj.data ?? {}, - summary: typeof obj.summary === 'string' ? obj.summary : '' - } - } -} - -function newKey(workspace: string, kind: LegacyKind, path: string): string { - return `userdraft/w/${workspace}/${kind}/${path}` -} - function listLocalStorageKeys(): string[] { const out: string[] = [] for (let i = 0; i < localStorage.length; i++) { @@ -139,16 +119,11 @@ function listLocalStorageKeys(): string[] { } /** - * Run the legacy → new-format migration. Idempotent: returns immediately if a - * previous run completed (signalled by `MIGRATION_FLAG`). - * - * The migration is workspace-scoped because the legacy keys had no notion of - * workspace — we treat the caller's current workspace as the owner of any - * surviving legacy entries. + * Remove the workspace-blind legacy autosave keys (see file header). Idempotent: + * returns immediately if a previous run completed (signalled by `MIGRATION_FLAG`). */ -export function migrateLegacyUserDrafts(workspace: string): void { +export function purgeLegacyUserDrafts(): void { if (typeof localStorage === 'undefined') return - if (!workspace) return if (localStorage.getItem(MIGRATION_FLAG) !== null) return try { @@ -157,32 +132,18 @@ export function migrateLegacyUserDrafts(workspace: string): void { if (!match) continue const raw = localStorage.getItem(key) if (raw == null) continue - - try { - const decoded = decodeLegacyState(raw) - if (!isPlausibleLegacyValue(match.newKind, decoded)) continue - const value = transformLegacyValue(match.newKind, decoded) - const target = newKey(workspace, match.newKind, match.path) - if (value !== undefined && localStorage.getItem(target) == null) { - // `lastWrittenAt` makes the migrated entry visible to - // `gcUserDrafts`. We stamp it as "now" so a freshly-migrated - // autosave gets the full retention window — sweeping it - // immediately on the first GC pass would lose work the - // legacy migration just rescued. - localStorage.setItem(target, JSON.stringify({ value, lastWrittenAt: Date.now() })) - } - localStorage.removeItem(key) - } catch (e) { - console.error('UserDraft legacy migration: failed to migrate', key, e) - } + // Only drop keys we can positively recognise as legacy Windmill + // drafts; leave unrelated or unparseable look-alikes in place. + if (!isPlausibleLegacyValue(match.newKind, decodeLegacyState(raw))) continue + localStorage.removeItem(key) } localStorage.setItem(MIGRATION_FLAG, new Date().toISOString()) } catch (e) { - console.error('UserDraft legacy migration: aborted', e) + console.error('UserDraft legacy purge: aborted', e) } } -/** Test-only: clear the sentinel so the migration can re-run. */ +/** Test-only: clear the sentinel so the purge can re-run. */ export function __resetUserDraftLegacyMigrationForTesting(): void { try { localStorage.removeItem(MIGRATION_FLAG) diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte index fa9bf2122d..4f6004ce2a 100644 --- a/frontend/src/routes/(root)/(logged)/+layout.svelte +++ b/frontend/src/routes/(root)/(logged)/+layout.svelte @@ -58,7 +58,7 @@ import GlobalSearchModal from '$lib/components/search/GlobalSearchModal.svelte' import MenuButton from '$lib/components/sidebar/MenuButton.svelte' import { loadProtectionRules } from '$lib/workspaceProtectionRules.svelte' - import { migrateLegacyUserDrafts } from '$lib/userDraftLegacyMigration' + import { purgeLegacyUserDrafts } from '$lib/userDraftLegacyMigration' import { migrateUserDraftsToDb } from '$lib/userDraftDbMigration' import DraftMigrationErrorModal from '$lib/components/DraftMigrationErrorModal.svelte' import { setContext, untrack } from 'svelte' @@ -435,16 +435,17 @@ $effect(() => { $workspaceStore && untrack(() => onLoad()) }) - // One-shot UserDraft migration chain. `migrateLegacyUserDrafts` folds - // the legacy `flow` / `app-…` / `rawapp-…` LS keys into the - // `userdraft/w/{ws}/{kind}/{path}` format; `migrateUserDraftsToDb` - // then pushes those onto the server-side draft table and clears LS - // on success. The order matters — the second step only sees what - // the first one normalized. + // One-shot UserDraft migration. `purgeLegacyUserDrafts` drops the oldest + // workspace-blind `flow` / `app-…` / `rawapp-…` LS autosave keys (they + // can't be attributed to a workspace, so promoting them would mis-file + // drafts). `migrateUserDraftsToDb` then pushes the workspace-scoped + // `userdraft/w/{ws}/{kind}/{path}` keys — written by the editor with the + // correct workspace — onto the server-side draft table, clearing LS on + // success. $effect(() => { if ($workspaceStore && $userStore) { untrack(() => { - migrateLegacyUserDrafts($workspaceStore!) + purgeLegacyUserDrafts() void migrateUserDraftsToDb() }) }