From a89b896ce5638f42f334055f9ffe6971b047aa84 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 11 Jul 2026 15:50:41 +0200 Subject: [PATCH] fix(frontend): mint draft path for new SDK builder items so autosave attaches (#10056) Fixes WIN-2159 Co-authored-by: Claude Opus 4.8 (1M context) --- .../src/lib/components/FlowWrapper.svelte | 21 +++++--- .../src/lib/components/ScriptWrapper.svelte | 16 ++++-- frontend/src/lib/components/script_builder.ts | 9 ++++ frontend/src/lib/draftAddRedirect.ts | 16 +++--- frontend/src/lib/mintDraftPath.test.ts | 50 +++++++++++++++++++ frontend/src/lib/mintDraftPath.ts | 42 ++++++++++++++++ frontend/src/lib/userDraft.svelte.ts | 27 +++++++--- 7 files changed, 155 insertions(+), 26 deletions(-) create mode 100644 frontend/src/lib/mintDraftPath.test.ts create mode 100644 frontend/src/lib/mintDraftPath.ts diff --git a/frontend/src/lib/components/FlowWrapper.svelte b/frontend/src/lib/components/FlowWrapper.svelte index 6eaf199ea7..2bd24f151a 100644 --- a/frontend/src/lib/components/FlowWrapper.svelte +++ b/frontend/src/lib/components/FlowWrapper.svelte @@ -5,6 +5,7 @@ import FlowBuilder from './FlowBuilder.svelte' import { usePageDraftSync } from './usePageDraftSync.svelte' import { workspaceStore } from '$lib/stores' + import { selectDraftStoragePath } from '$lib/mintDraftPath' import type { OpenFlow } from '$lib/gen' let { @@ -28,13 +29,19 @@ // Stable per-user draft storage key. Captured once so editing the flow's path // (which lives in `draft_path`, not the storage key) can't re-key the autosave // handle and orphan the draft. Mirrors the full-page editor keying on the URL - // path; falls back through the SDK's path inputs. - const draftStoragePath = untrack( - () => - props.initialPath || - props.pathStoreInit || - (oldFlowStore.val as { path?: string } | undefined)?.path || - '' + // path; falls back through the SDK's path inputs. For a brand-new flow with no + // caller path this mints a `u//draft_` key — the SDK equivalent of + // the `/flows/add` redirect — so autosave attaches instead of the handle + // detaching (local-only, never POSTs). + const draftStoragePath = untrack(() => + selectDraftStoragePath({ + providedPaths: [ + props.initialPath, + props.pathStoreInit, + (oldFlowStore.val as { path?: string } | undefined)?.path + ], + isNewItem: !!props.newFlow + }) ) // Reuse the full-page flow editor's draft orchestration so the SDK gets diff --git a/frontend/src/lib/components/ScriptWrapper.svelte b/frontend/src/lib/components/ScriptWrapper.svelte index 7fafdecc83..ff8fc8b2f5 100644 --- a/frontend/src/lib/components/ScriptWrapper.svelte +++ b/frontend/src/lib/components/ScriptWrapper.svelte @@ -5,12 +5,22 @@ import type { ScriptBuilderProps } from './script_builder' import { usePageDraftSync } from './usePageDraftSync.svelte' import { workspaceStore } from '$lib/stores' + import { selectDraftStoragePath } from '$lib/mintDraftPath' - let { script: oldScript, disableAi, ...props }: ScriptBuilderProps = $props() + let { script: oldScript, disableAi, newScript, ...props }: ScriptBuilderProps = $props() // Stable per-user draft storage key. Mirrors the full-page editor keying on - // the URL path; falls back through the SDK's path inputs. - const draftStoragePath = untrack(() => props.initialPath || oldScript?.path || '') + // the URL path; falls back through the SDK's path inputs. For a brand-new + // script with no caller path this mints a `u//draft_` key — the + // SDK equivalent of the `/scripts/add` redirect — so autosave attaches instead + // of the handle detaching (local-only, never POSTs). Captured once (untrack) + // so editing the path field can't re-key and orphan the draft. + const draftStoragePath = untrack(() => + selectDraftStoragePath({ + providedPaths: [props.initialPath, oldScript?.path], + isNewItem: !!newScript + }) + ) // Reuse the full-page script editor's draft orchestration (same as the flow // SDK) so the SDK gets autosave + the AutosaveIndicator (gated by ScriptBuilder diff --git a/frontend/src/lib/components/script_builder.ts b/frontend/src/lib/components/script_builder.ts index 0fbd24d904..0609c6737d 100644 --- a/frontend/src/lib/components/script_builder.ts +++ b/frontend/src/lib/components/script_builder.ts @@ -15,6 +15,15 @@ export interface ScriptBuilderProps { disableAi?: boolean fullyLoaded?: boolean initialPath?: string + /** + * Wrapper-only signal (consumed by `ScriptWrapper`, not `ScriptBuilder`): + * this editor is mounting a brand-new script. When set and no caller path + * is provided, the wrapper mints a `u//draft_` storage path so + * autosave attaches — mirrors what the `/scripts/add` route does before the + * full-page editor mounts. Left unset for read-only / pathless views so + * autosave stays intentionally detached. + */ + newScript?: boolean /** * Path the route's `UserDraft.use('script', ...)` * handle is keyed by. Distinct from `initialPath` for new drafts — diff --git a/frontend/src/lib/draftAddRedirect.ts b/frontend/src/lib/draftAddRedirect.ts index 2f9e381ed1..242893f782 100644 --- a/frontend/src/lib/draftAddRedirect.ts +++ b/frontend/src/lib/draftAddRedirect.ts @@ -1,25 +1,21 @@ import { redirect } from '@sveltejs/kit' import { base } from '$app/paths' -import { getUsernameForNamespace } from '$lib/userNamespace' -import { randomUUID } from '$lib/utils/uuid' +import { mintDraftPath } from '$lib/mintDraftPath' /** * Shared `load` for every `/{scripts,flows,apps,apps_raw}/add` route. Doing the * redirect in `load` (not `onMount`) avoids painting a blank frame first. * - * Mints a fresh `u//draft_` path and 307s to + * Mints a fresh `u//draft_` path (via the shared `mintDraftPath`, + * so the SDK builder wrappers stay in lockstep) and 307s to * `{base}/{editPrefix}/?new_draft=true&`. `new_draft=true` * tells the edit route to seed an empty editor instead of 404-ing. The hash is * carried over too — fork / handler-template buttons encode a payload into it - * that the edit route's `new_draft` branch consumes. `randomUUID` not - * `crypto.randomUUID` (WebCrypto is absent on non-secure origins). + * that the edit route's `new_draft` branch consumes. */ export function makeDraftAddLoad(editPrefix: string) { return ({ url }: { url: URL }) => { - const username = getUsernameForNamespace() - // Underscores not dashes — path segments are `[a-zA-Z0-9_]` words and - // downstream consumers treat `-` as foreign. - const uuid = randomUUID().replaceAll('-', '_') + const path = mintDraftPath() const params = new URLSearchParams(url.searchParams) params.set('new_draft', 'true') // `url.hash` is unavailable in `load`; read `window.location` instead @@ -27,6 +23,6 @@ export function makeDraftAddLoad(editPrefix: string) { // points at the PREVIOUS page, but every hash-payload producer arrives // as a full page load, so the hash is correct. const hash = typeof window !== 'undefined' ? window.location.hash : '' - redirect(307, `${base}/${editPrefix}/u/${username}/draft_${uuid}?${params.toString()}${hash}`) + redirect(307, `${base}/${editPrefix}/${path}?${params.toString()}${hash}`) } } diff --git a/frontend/src/lib/mintDraftPath.test.ts b/frontend/src/lib/mintDraftPath.test.ts new file mode 100644 index 0000000000..10569f0f01 --- /dev/null +++ b/frontend/src/lib/mintDraftPath.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect, vi } from 'vitest' + +vi.mock('$lib/userNamespace', () => ({ + getUsernameForNamespace: () => 'alice' +})) + +import { mintDraftPath, selectDraftStoragePath } from './mintDraftPath' + +const MINTED = /^u\/alice\/draft_[0-9a-f_]+$/ + +describe('mintDraftPath', () => { + it('mints a non-empty u//draft_ path', () => { + const path = mintDraftPath() + expect(path).not.toBe('') + expect(path).toMatch(MINTED) + }) + + it('uses underscores, never dashes (path segments are word chars)', () => { + expect(mintDraftPath()).not.toContain('-') + }) + + it('is unique per call', () => { + expect(mintDraftPath()).not.toBe(mintDraftPath()) + }) +}) + +describe('selectDraftStoragePath', () => { + it('mints a non-empty path for a new item with no caller path', () => { + // Regression: SDK create wrappers keyed autosave under '' → detached → + // silently never POSTed. A new item must get a real, mintable key. + const path = selectDraftStoragePath({ providedPaths: [undefined, ''], isNewItem: true }) + expect(path).toMatch(MINTED) + }) + + it('honors the first caller-provided path over minting (SDK stopgap wins)', () => { + expect( + selectDraftStoragePath({ providedPaths: ['u/bob/given', undefined], isNewItem: true }) + ).toBe('u/bob/given') + }) + + it('falls through empties to the first non-empty provided path', () => { + expect( + selectDraftStoragePath({ providedPaths: ['', undefined, 'u/bob/existing'], isNewItem: false }) + ).toBe('u/bob/existing') + }) + + it('stays detached ("") for a non-new item with no path (read-only view)', () => { + expect(selectDraftStoragePath({ providedPaths: [undefined, ''], isNewItem: false })).toBe('') + }) +}) diff --git a/frontend/src/lib/mintDraftPath.ts b/frontend/src/lib/mintDraftPath.ts new file mode 100644 index 0000000000..5df9c8838a --- /dev/null +++ b/frontend/src/lib/mintDraftPath.ts @@ -0,0 +1,42 @@ +import { getUsernameForNamespace } from '$lib/userNamespace' +import { randomUUID } from '$lib/utils/uuid' + +/** + * Mint a fresh `u//draft_` storage path for a brand-new + * editor item. Shared by the `/{scripts,flows,apps,apps_raw}/add` route + * redirects (`makeDraftAddLoad`) and the SDK builder wrappers + * (`ScriptWrapper` / `FlowWrapper`) so the two can't diverge on format — + * both need the autosave handle keyed under a real, unique path or the + * draft detaches (local-only) and never POSTs. + * + * Underscores not dashes — path segments are `[a-zA-Z0-9_]` words and + * downstream consumers treat `-` as foreign. `randomUUID` not + * `crypto.randomUUID` (WebCrypto is absent on non-secure origins). + */ +export function mintDraftPath(): string { + const username = getUsernameForNamespace() + const uuid = randomUUID().replaceAll('-', '_') + return `u/${username}/draft_${uuid}` +} + +/** + * Resolve the autosave storage key for an SDK builder wrapper. The first + * caller-provided path wins (a consumer that supplies its own path — including + * the temporary React-SDK stopgap — keeps it); otherwise, for a brand-new + * editable item, mint a fresh draft path so autosave attaches. Returns `''` + * (a detached, local-only handle that never POSTs) when there is no path and + * the item isn't a new editable one — the intentionally-pathless read-only case. + * + * Shared by `ScriptWrapper` / `FlowWrapper` so the two can't diverge on this + * precedence. Callers MUST capture the result once (`untrack`) so editing a + * path field later can't re-key the handle and orphan the draft. + */ +export function selectDraftStoragePath(opts: { + providedPaths: (string | undefined)[] + isNewItem: boolean +}): string { + for (const p of opts.providedPaths) { + if (p) return p + } + return opts.isNewItem ? mintDraftPath() : '' +} diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index a81c223fb1..b9392cee9a 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -746,8 +746,8 @@ function acquireEntry( existing.count++ return } - // Seed the cell with `defaultValue` (deep-cloned). Swallowed by - // `skipNextWrite` below — it never POSTs. + // Seed the cell with `defaultValue` (deep-cloned). The mirror below anchors + // its baseline to this seed so it never POSTs the template. const seed = defaultValue !== undefined ? snapshotDraftValue(defaultValue) : undefined const cell = $state<{ val: unknown }>({ val: seed }) const stateRef = cell as DraftState @@ -780,14 +780,29 @@ function acquireEntry( // `cell.val` alone only subscribes to the proxy root. // // `lastSerialized` + `skipNextWrite` dedup no-op updates and treat - // the FIRST change after mount as the seed/restore (no POST), so - // landing on `?new_draft` doesn't sync until the user edits. + // the seed/restore as no-POST, so landing on `?new_draft` doesn't + // sync until the user edits. + // + // The swallow is anchored to the seed VALUE, not "first change after + // mount": when a `defaultValue` seeded the cell, `lastSerialized` + // starts at its serialization so a first mirror run that still equals + // the seed is a genuine no-op, while a first run that already DIFFERS + // is the user's edit and must POST. That closes a create-path race — + // the mirror is created in a deferred microtask (see below) and the + // handle re-keys detached→acquired as the workspace resolves, so the + // seed and the user's first edit can land before the mirror's first + // run and coalesce into it; a blind "swallow the first change" would + // eat that edit. + // + // With no in-hand seed (page editors load & assign the value AFTER + // mount), arm the blind first-write swallow so that post-load + // assignment doesn't POST. // // `cell.val === undefined` is the delete signal (`value: null`). // `skipNextSync` lets callers that already POSTed (`discard`, // `remove`) suppress the duplicate fire from their own write. - let lastSerialized: string | undefined = undefined - let skipNextWrite = true + let lastSerialized: string | undefined = seed === undefined ? undefined : JSON.stringify(seed) + let skipNextWrite = seed === undefined $effect(() => { const val = cell.val if (val !== undefined) readFieldsRecursively(val)