From d3b87cf70b98f916f811bfa5d7d026aa5222b984 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 11:15:11 +0200 Subject: [PATCH] fix: a legacy draft occupies its destination, a superseded opening writes nothing, and a loaded flow draft keeps no base it lacks Co-Authored-By: Claude Opus 5 (1M context) --- ...3480afb5d12a05791832d91ca27ab2be3370ad19e46fc1a.json} | 4 ++-- ...12c618f48f38d99f84311ac3297f04bc19425a4bddceb33.json} | 4 ++-- backend/windmill-api/openapi.yaml | 2 +- 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 +++++++-- .../(root)/(logged)/flows/edit/[...path]/+page.svelte | 5 ++++- 8 files changed, 36 insertions(+), 14 deletions(-) rename backend/.sqlx/{query-4a125e25142bdfb71f011ffe08e7d0be51e007b2e117af240cd1cf035a6ef0ff.json => query-3e3651d23d759a4cb3480afb5d12a05791832d91ca27ab2be3370ad19e46fc1a.json} (72%) rename backend/.sqlx/{query-e27e447e4d9607fe280807e396049bbe31e4e938d9f53eeb659cdc49f9be8f2c.json => query-cb135f028eb579ee912c618f48f38d99f84311ac3297f04bc19425a4bddceb33.json} (88%) diff --git a/backend/.sqlx/query-4a125e25142bdfb71f011ffe08e7d0be51e007b2e117af240cd1cf035a6ef0ff.json b/backend/.sqlx/query-3e3651d23d759a4cb3480afb5d12a05791832d91ca27ab2be3370ad19e46fc1a.json similarity index 72% rename from backend/.sqlx/query-4a125e25142bdfb71f011ffe08e7d0be51e007b2e117af240cd1cf035a6ef0ff.json rename to backend/.sqlx/query-3e3651d23d759a4cb3480afb5d12a05791832d91ca27ab2be3370ad19e46fc1a.json index 6c91856635..3e69d0950d 100644 --- a/backend/.sqlx/query-4a125e25142bdfb71f011ffe08e7d0be51e007b2e117af240cd1cf035a6ef0ff.json +++ b/backend/.sqlx/query-3e3651d23d759a4cb3480afb5d12a05791832d91ca27ab2be3370ad19e46fc1a.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT\n (SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ::text = ANY($6::text[]) AND email = $4 LIMIT 1) as \"at_target\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email IS NULL) as \"legacy!\" ", + "query": "SELECT\n (SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ::text = ANY($6::text[]) AND (email = $4 OR email IS NULL)\n LIMIT 1) as \"at_target\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email IS NULL) as \"legacy!\" ", "describe": { "columns": [ { @@ -69,5 +69,5 @@ null ] }, - "hash": "4a125e25142bdfb71f011ffe08e7d0be51e007b2e117af240cd1cf035a6ef0ff" + "hash": "3e3651d23d759a4cb3480afb5d12a05791832d91ca27ab2be3370ad19e46fc1a" } diff --git a/backend/.sqlx/query-e27e447e4d9607fe280807e396049bbe31e4e938d9f53eeb659cdc49f9be8f2c.json b/backend/.sqlx/query-cb135f028eb579ee912c618f48f38d99f84311ac3297f04bc19425a4bddceb33.json similarity index 88% rename from backend/.sqlx/query-e27e447e4d9607fe280807e396049bbe31e4e938d9f53eeb659cdc49f9be8f2c.json rename to backend/.sqlx/query-cb135f028eb579ee912c618f48f38d99f84311ac3297f04bc19425a4bddceb33.json index 376f7ec61c..335ed9035d 100644 --- a/backend/.sqlx/query-e27e447e4d9607fe280807e396049bbe31e4e938d9f53eeb659cdc49f9be8f2c.json +++ b/backend/.sqlx/query-cb135f028eb579ee912c618f48f38d99f84311ac3297f04bc19425a4bddceb33.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "UPDATE draft\n SET path = $3,\n -- Both path keys, not just the typed one: the editors mirror the\n -- typed path into the other while it differs from the row's path,\n -- and the loaders prefer the mirror — left naming the old location\n -- it un-does this move on the next save. `create_missing = false`\n -- on both, so a draft carrying only one keeps only one.\n value = to_json(\n jsonb_set(\n jsonb_set(\n CASE WHEN $7::text IS NULL THEN to_jsonb(value)\n ELSE jsonb_set(to_jsonb(value), ARRAY['summary'], to_jsonb($7::text))\n END,\n ARRAY[$5::text], to_jsonb($3::text), false\n ),\n ARRAY[$8::text], to_jsonb($3::text), false\n )\n )\n WHERE workspace_id = $1\n AND path = $2\n AND typ = $4\n AND email = $6\n -- A pre-sanitizer NUL escape makes `to_jsonb` raise 22P05. Excluded\n -- here so the statement can't 500; reported below instead. Unlike the\n -- passive carry, rewriting the value IS this operation, so skipping it\n -- silently would move the row and leave its typed path stale.\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) = 0\n -- Skipped on a summary-only edit, where the \"target\" row is this\n -- row and the guard would refuse the update against itself.\n AND ($2 = $3 OR NOT EXISTS (\n SELECT 1 FROM draft o\n WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ::text = ANY($9::text[])\n AND o.email = $6\n ))\n RETURNING id", + "query": "UPDATE draft\n SET path = $3,\n -- Both path keys, not just the typed one: the editors mirror the\n -- typed path into the other while it differs from the row's path,\n -- and the loaders prefer the mirror — left naming the old location\n -- it un-does this move on the next save. `create_missing = false`\n -- on both, so a draft carrying only one keeps only one.\n value = to_json(\n jsonb_set(\n jsonb_set(\n CASE WHEN $7::text IS NULL THEN to_jsonb(value)\n ELSE jsonb_set(to_jsonb(value), ARRAY['summary'], to_jsonb($7::text))\n END,\n ARRAY[$5::text], to_jsonb($3::text), false\n ),\n ARRAY[$8::text], to_jsonb($3::text), false\n )\n )\n WHERE workspace_id = $1\n AND path = $2\n AND typ = $4\n AND email = $6\n -- A pre-sanitizer NUL escape makes `to_jsonb` raise 22P05. Excluded\n -- here so the statement can't 500; reported below instead. Unlike the\n -- passive carry, rewriting the value IS this operation, so skipping it\n -- silently would move the row and leave its typed path stale.\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) = 0\n -- Skipped on a summary-only edit, where the \"target\" row is this\n -- row and the guard would refuse the update against itself.\n AND ($2 = $3 OR NOT EXISTS (\n SELECT 1 FROM draft o\n WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ::text = ANY($9::text[])\n -- The legacy row counts: a deploy at that path wipes it together with\n -- the caller's, so parking a second draft there discards edits the\n -- caller never saw.\n AND (o.email = $6 OR o.email IS NULL)\n ))\n RETURNING id", "describe": { "columns": [ { @@ -60,5 +60,5 @@ false ] }, - "hash": "e27e447e4d9607fe280807e396049bbe31e4e938d9f53eeb659cdc49f9be8f2c" + "hash": "cb135f028eb579ee912c618f48f38d99f84311ac3297f04bc19425a4bddceb33" } diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 41b52461df..29828c2f09 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -9805,7 +9805,7 @@ paths: format: date-time path: type: string - description: "`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." + description: "`saved` only, upsert or delete: where the write landed. Differs from the URL path when the item had moved away from it; the editor follows it there." required: [status, current_timestamp] /w/{workspace}/drafts/move/{kind}/{path}: diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index dbefcdd468..abd0d80205 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -739,7 +739,10 @@ async fn move_draft( AND ($2 = $3 OR NOT EXISTS ( SELECT 1 FROM draft o WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ::text = ANY($9::text[]) - AND o.email = $6 + -- The legacy row counts: a deploy at that path wipes it together with + -- the caller's, so parking a second draft there discards edits the + -- caller never saw. + AND (o.email = $6 OR o.email IS NULL) )) RETURNING id"#, &w_id, @@ -771,7 +774,8 @@ async fn move_draft( let row = sqlx::query!( r#"SELECT (SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3 - AND typ::text = ANY($6::text[]) AND email = $4 LIMIT 1) as "at_target", + AND typ::text = ANY($6::text[]) AND (email = $4 OR email IS NULL) + LIMIT 1) as "at_target", EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5 AND typ = $2 AND email = $4 AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0 diff --git a/frontend/src/lib/components/FlowBuilder.svelte b/frontend/src/lib/components/FlowBuilder.svelte index 45eab861dd..fdef732939 100644 --- a/frontend/src/lib/components/FlowBuilder.svelte +++ b/frontend/src/lib/components/FlowBuilder.svelte @@ -472,7 +472,7 @@ } } } - async function syncWithDeployed() { + async function syncWithDeployed(opening?: number) { const flow = await FlowService.getFlowByPath({ workspace: opWorkspace!, // The draft row's own path, not `initialPath` — the route re-seeds that from @@ -482,6 +482,9 @@ path: userDraftPath || initialPath, withStarredInfo: true }) + // A superseded opening must not write these: the current one would then render + // and offer Take latest against the older head. + if (opening != null && opening !== diffOpening) return deployedValue = replaceFalseWithUndefined({ ...flow, edited_at: undefined, @@ -1170,8 +1173,10 @@ export async function openDiffDrawer() { const opening = ++diffOpening if (!savedFlow) return - await syncWithDeployed() + await syncWithDeployed(opening) const currentDraftTriggers = structuredClone(triggersState.getDraftTriggersSnapshot()) + // Blanking the drawer belongs to the opening that will fill it. + if (opening !== diffOpening) return diffDrawer?.openDrawer() const currentFlow = flowStore.val const versions = await deployedVersionOptions() diff --git a/frontend/src/lib/components/ScriptBuilder.svelte b/frontend/src/lib/components/ScriptBuilder.svelte index 103cef5919..89e5079a3e 100644 --- a/frontend/src/lib/components/ScriptBuilder.svelte +++ b/frontend/src/lib/components/ScriptBuilder.svelte @@ -616,7 +616,7 @@ } } - async function syncWithDeployed() { + async function syncWithDeployed(opening?: number) { const latestScript = await ScriptService.getScriptByPath({ workspace: opWorkspace!, // The draft row's own path, not `initialPath` — that one tracks the path @@ -627,6 +627,9 @@ withStarredInfo: true }) + // A superseded opening must not write these: the current one would then render + // and offer Take latest against the older head. + if (opening != null && opening !== diffOpening) return deployedValue = replaceFalseWithUndefined({ ...latestScript, workspace_id: undefined, @@ -879,7 +882,7 @@ if (!savedScript) { return } - await syncWithDeployed() + await syncWithDeployed(opening) const currentDraftTriggers = structuredClone(triggersState.getDraftTriggersSnapshot()) @@ -896,6 +899,8 @@ } if (current.assets && !current.assets.length) delete current.assets + // Blanking the drawer belongs to the opening that will fill it. + if (opening !== diffOpening) return diffDrawer?.openDrawer() const headHash = (deployed as { hash?: string } | undefined)?.hash const versions = await deployedVersionOptions(headHash) diff --git a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte index 4826db8645..49d475da9d 100644 --- a/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte +++ b/frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte @@ -397,13 +397,16 @@ } } - async function syncWithDeployed() { + async function syncWithDeployed(opening?: number) { const deployedApp = await AppService.getAppByPath({ workspace: opWorkspace!, path: appPath!, withStarredInfo: true }) + // A superseded opening must not write these: the current one would then render + // and offer Take latest against the older head. + if (opening != null && opening !== diffOpening) return deployedBy = deployedApp.created_by const shownVersions = (deployedApp as { versions?: number[] }).versions deployedVersionShown = Array.isArray(shownVersions) @@ -461,8 +464,10 @@ } // deployedValue should be syncronized when we open Diff - await syncWithDeployed() + await syncWithDeployed(opening) + // Blanking the drawer belongs to the opening that will fill it. + if (opening !== diffOpening) return diffDrawer?.openDrawer() const versions = await deployedVersionOptions() if (opening !== diffOpening) return 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 13cb0af677..4d240cc24b 100644 --- a/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/flows/edit/[...path]/+page.svelte @@ -405,9 +405,12 @@ : effectiveFlow flow = flowToRender if (pendingLoad) { - // Their draft's base, not ours; see /scripts/edit. + // Their draft's base, not ours; see /scripts/edit. With none, the merge above + // would hand it the deployed one and the next save would persist that as its + // base, so it is dropped rather than inherited. const theirs = (pendingLoad.value as { version_id?: number })?.version_id draftBaseVersion = theirs != null ? String(theirs) : undefined + if (theirs == null) delete (flowToRender as any).version_id } if (pendingLoad && hasOwnDraft) { OtherUserDraftLoad.beginOverlay({