From 493448b2ce670d2db6e4445a22ef701ba3b48471 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 10:31:49 +0200 Subject: [PATCH] fix: a routed discard names where it landed, a superseded drawer opening is dropped, and a loaded draft keeps its base in every editor Co-Authored-By: Claude Opus 5 (1M context) --- backend/windmill-api/src/drafts.rs | 8 +++++--- frontend/src/lib/components/FlowBuilder.svelte | 9 ++++++++- frontend/src/lib/components/ScriptBuilder.svelte | 9 ++++++++- .../lib/components/raw_apps/RawAppEditorHeader.svelte | 9 ++++++++- frontend/src/lib/userDraftDbSyncer.svelte.ts | 5 +++-- .../(root)/(logged)/apps/edit/[...path]/+page.svelte | 8 +++++++- .../(root)/(logged)/flows/edit/[...path]/+page.svelte | 5 +++++ .../(root)/(logged)/scripts/edit/[...path]/+page.svelte | 4 ++++ 8 files changed, 48 insertions(+), 9 deletions(-) diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index 2dc1ef8b4a..dbefcdd468 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -321,8 +321,8 @@ pub struct SaveDraftResponse { /// On `saved`: when the change was applied (client remembers it as the /// next `last_sync`). On `conflict`: the existing row's `created_at`. pub current_timestamp: chrono::DateTime, - /// `saved` upserts only: where the draft is. Differs from the URL path when - /// the item had moved away from it; the editor follows it there. + /// `saved` only: where the write landed. Differs from the URL path when the item + /// had moved away from it; the editor follows it there. #[serde(skip_serializing_if = "Option::is_none")] pub path: Option, } @@ -538,7 +538,9 @@ async fn update_draft( ) .fetch_optional(&db) .await? - .map(|ts| (ts, None)) + // Named for the same reason an upsert is: the editor that discarded is still on + // the path the item left, and reloading there would land on nothing. + .map(|ts| (ts, moved_to.clone())) }; if let Some((ts, path)) = applied { diff --git a/frontend/src/lib/components/FlowBuilder.svelte b/frontend/src/lib/components/FlowBuilder.svelte index 78a1eaf47f..45eab861dd 100644 --- a/frontend/src/lib/components/FlowBuilder.svelte +++ b/frontend/src/lib/components/FlowBuilder.svelte @@ -1163,17 +1163,24 @@ } } + /** Bumped per drawer opening: the fetches below are awaited, so a reopen (or a + * path change) while they run must not have the older one land last. */ + let diffOpening = 0 + export async function openDiffDrawer() { + const opening = ++diffOpening if (!savedFlow) return await syncWithDeployed() const currentDraftTriggers = structuredClone(triggersState.getDraftTriggersSnapshot()) diffDrawer?.openDrawer() const currentFlow = flowStore.val + const versions = await deployedVersionOptions() + if (opening !== diffOpening) return diffDrawer?.setDiff({ mode: 'normal', deployed: deployedValue ?? savedFlow, deployedLabel, - versions: await deployedVersionOptions(), + versions, onTakeLatest, draftBase: draftBaseVersion, deployedHead: deployedVersionShown != null ? String(deployedVersionShown) : undefined, diff --git a/frontend/src/lib/components/ScriptBuilder.svelte b/frontend/src/lib/components/ScriptBuilder.svelte index 61c3928a1d..103cef5919 100644 --- a/frontend/src/lib/components/ScriptBuilder.svelte +++ b/frontend/src/lib/components/ScriptBuilder.svelte @@ -870,7 +870,12 @@ } } + /** Bumped per drawer opening: the fetches below are awaited, so a reopen (or a + * path change) while they run must not have the older one land last. */ + let diffOpening = 0 + export async function openDiffDrawer() { + const opening = ++diffOpening if (!savedScript) { return } @@ -893,11 +898,13 @@ diffDrawer?.openDrawer() const headHash = (deployed as { hash?: string } | undefined)?.hash + const versions = await deployedVersionOptions(headHash) + if (opening !== diffOpening) return diffDrawer?.setDiff({ mode: 'normal', deployed, deployedLabel: deployedVersionLabel(deployed), - versions: await deployedVersionOptions(headHash), + versions, onTakeLatest, draftBase: draftBaseHash, deployedHead: headHash, diff --git a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte index f5e6d3d781..4826db8645 100644 --- a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte @@ -450,7 +450,12 @@ } } + /** Bumped per drawer opening: the fetches below are awaited, so a reopen (or a + * path change) while they run must not have the older one land last. */ + let diffOpening = 0 + export async function openDiffDrawer() { + const opening = ++diffOpening if (!savedApp) { return } @@ -459,10 +464,12 @@ await syncWithDeployed() diffDrawer?.openDrawer() + const versions = await deployedVersionOptions() + if (opening !== diffOpening) return diffDrawer?.setDiff({ mode: 'normal', deployed: deployedValue ?? stripRawAppDiffNoise(savedApp), - versions: await deployedVersionOptions(), + versions, onTakeLatest, draftBase: draftBaseVersion, deployedHead: deployedVersionShown != null ? String(deployedVersionShown) : undefined, diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts index e4d4737d40..25125a74e4 100644 --- a/frontend/src/lib/userDraftDbSyncer.svelte.ts +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -340,9 +340,10 @@ async function postSave(opts: UserDraftDbSyncerSaveOpts): Promise { // cached state the same way an upsert does. Listener errors must never // make a committed save read as failed. notifyAnySaved({ workspace: opts.workspace, itemKind: opts.itemKind, path: opts.path }) - // The item had moved and the save landed where its drafts went. Last, + // The item had moved and the write landed where its drafts went — a discard + // included, since the editor that sent it is on a path the item has left. Last, // so the editor that reacts (by leaving this path) sees a settled key. - if (opts.value !== null && resp.path && resp.path !== opts.path) { + if (resp.path && resp.path !== opts.path) { const listeners = relocationListeners.get(key) if (listeners) for (const l of [...listeners]) l(resp.path) } diff --git a/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte index 5570999812..f1e309ba14 100644 --- a/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/apps/edit/[...path]/+page.svelte @@ -307,6 +307,11 @@ } if (pendingLoad) { backendApp = { ...backendApp, value: pendingLoad.value as App } as typeof backendApp + // Their draft's base, not ours: the prompt and the deploy guard read it, and + // deploying their content on our base would claim a version it never forked + // from. See /scripts/edit. + const theirs = (pendingLoad.value as App)?.parent_version + draftBaseVersion = theirs != null ? String(theirs) : undefined if (hasOwnDraft) { // AppEditor `migrateApp`s the value in place on mount (see its // `migratedDeployedBaseline`), so the draft cell settles to the @@ -334,7 +339,8 @@ // draft value. An existing own draft already carries it (preserved by the // value swap above). `parent_version` is in DRAFT_COMPARE_IGNORED_FIELDS, so it // never trips the autosave no-op / "unsaved changes" comparison. - if (!hasOwnDraft && !backendApp.no_deployed && backendApp.value) { + // Not after loading a teammate's draft either: that value carries their base. + if (!hasOwnDraft && !pendingLoad && !backendApp.no_deployed && backendApp.value) { const versions = (backendApp as { versions?: number[] }).versions const head = Array.isArray(versions) ? versions[versions.length - 1] : undefined if (head != null) (backendApp.value as App).parent_version = head diff --git a/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte index 6ba4a7e5ee..13cb0af677 100644 --- a/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte @@ -404,6 +404,11 @@ ? ({ ...deployedFlow, ...(pendingLoad.value as object) } as Flow) : effectiveFlow flow = flowToRender + if (pendingLoad) { + // Their draft's base, not ours; see /scripts/edit. + const theirs = (pendingLoad.value as { version_id?: number })?.version_id + draftBaseVersion = theirs != null ? String(theirs) : undefined + } if (pendingLoad && hasOwnDraft) { OtherUserDraftLoad.beginOverlay({ workspace: $workspaceStore!, diff --git a/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte index f9daa3ecae..31f82c0d48 100644 --- a/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte @@ -389,6 +389,10 @@ ...(pendingLoad.value as object), parent_hash: (pendingLoad.value as { parent_hash?: string })?.parent_hash ?? parentHash } as EditableScript + // Their draft's base, not ours: the prompt and the deploy guard read these, + // and deploying their content on our base would claim a version it never + // forked from. + draftBaseHash = loadedValue.parent_hash if (hasOwnDraft) { OtherUserDraftLoad.beginOverlay({ workspace: $workspaceStore!,