fix: the drawer owns its opening, a loaded script draft keeps no base it lacks, and a legacy occupant says who can clear it

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

backend/.sqlx/query-07247926f7acb6578cd0edd021e56b0d0cadbc506823f6f2051f8764bf727fb8.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 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!\" ",
+  "query": "SELECT\n                 -- Own row first: with both an own and a legacy row at the destination,\n                 -- a bare LIMIT 1 would name an arbitrary one and the two need different\n                 -- remedies (discard your own vs. ask an admin).\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                  ORDER BY email NULLS LAST LIMIT 1) as \"at_target\",\n                 EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3\n                        AND typ::text = ANY($6::text[]) AND email = $4) as \"at_target_own!\",\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": [
       {
@@ -10,11 +10,16 @@
       },
       {
         "ordinal": 1,
-        "name": "poisoned!",
+        "name": "at_target_own!",
         "type_info": "Bool"
       },
       {
         "ordinal": 2,
+        "name": "poisoned!",
+        "type_info": "Bool"
+      },
+      {
+        "ordinal": 3,
         "name": "legacy!",
         "type_info": "Bool"
       }
@@ -64,10 +69,11 @@
       ]
     },
     "nullable": [
+      null,
       null,
       null,
       null
     ]
   },
-  "hash": "3e3651d23d759a4cb3480afb5d12a05791832d91ca27ab2be3370ad19e46fc1a"
+  "hash": "07247926f7acb6578cd0edd021e56b0d0cadbc506823f6f2051f8764bf727fb8"
 }

backend/tests/drafts_move_taken.rs

@@ -74,6 +74,59 @@ async fn test_rename_onto_a_draft_is_refused(db: Pool<Postgres>) -> anyhow::Resu
     Ok(())
 }
 
+/// A legacy (ownerless) draft occupies its path too: a deploy there deletes it together
+/// with the caller's own row, so a move that parks a second draft beside it would discard
+/// edits the caller never saw. Only an admin can clear it, so the refusal says so.
+#[sqlx::test(fixtures("base", "drafts_move_taken"))]
+async fn test_draft_move_refuses_a_legacy_destination(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+
+    let resp = reqwest::Client::new()
+        .post(format!(
+            "http://localhost:{port}/api/w/test-workspace/drafts/move/script/u/test-user/mvtaken_b"
+        ))
+        .header("Authorization", "Bearer SECRET_TOKEN")
+        .json(&json!({ "new_path": "u/test-user/mvtaken_legacy" }))
+        .send()
+        .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(status, 400, "move onto a legacy draft was allowed: {body}");
+    assert!(
+        body.contains("legacy workspace draft") && body.contains("workspace admin"),
+        "the refusal did not point at the one remedy: {body}"
+    );
+
+    // Both rows stayed where they were: the caller's own, and the legacy one the list
+    // synthesizes under the caller's name.
+    let list: Vec<Value> = reqwest::Client::new()
+        .get(format!(
+            "http://localhost:{port}/api/w/test-workspace/drafts/list"
+        ))
+        .header("Authorization", "Bearer SECRET_TOKEN")
+        .send()
+        .await?
+        .json()
+        .await?;
+    let mut at = list
+        .iter()
+        .filter_map(|d| Some((d["kind"].as_str()?, d["path"].as_str()?)))
+        .filter(|(_, p)| p.starts_with("u/test-user/mvtaken_b") || p.ends_with("mvtaken_legacy"))
+        .collect::<Vec<_>>();
+    at.sort();
+    assert_eq!(
+        at,
+        vec![
+            ("script", "u/test-user/mvtaken_b"),
+            ("script", "u/test-user/mvtaken_legacy")
+        ],
+        "{list:?}"
+    );
+    Ok(())
+}
+
 /// A classic app and a raw app deploy into the same table, so a draft-only move onto
 /// the other kind's draft must be refused: deploying either path afterwards deletes
 /// the caller's drafts of both kinds, taking the loser's item with it.
@@ -93,7 +146,10 @@ async fn test_draft_move_refuses_the_other_app_kind(db: Pool<Postgres>) -> anyho
         .await?;
     let status = resp.status();
     let body = resp.text().await?;
-    assert_eq!(status, 400, "move onto a classic app draft was allowed: {body}");
+    assert_eq!(
+        status, 400,
+        "move onto a classic app draft was allowed: {body}"
+    );
     assert!(
         body.contains("already have a draft at 'u/test-user/mvtaken_app' (app)"),
         "the refusal did not name the occupying kind: {body}"

backend/tests/fixtures/drafts_move_taken.sql

@@ -16,6 +16,13 @@ VALUES ('test-workspace', 'u/test-user/mvtaken_b', 'script',
         '{"path": "u/test-user/mvtaken_b", "summary": "B", "content": ""}',
         'test@windmill.dev');
 
+-- An ownerless (pre per-user drafts) row at a third path: a deploy there deletes it
+-- together with the caller's, so it occupies the path just as an own draft does.
+INSERT INTO draft (workspace_id, path, typ, value, email)
+VALUES ('test-workspace', 'u/test-user/mvtaken_legacy', 'script',
+        '{"path": "u/test-user/mvtaken_legacy", "summary": "legacy", "content": ""}',
+        NULL);
+
 -- A draft-only classic app and a draft-only raw app of the same owner. They share
 -- the `app` table, so one occupies the other's path.
 INSERT INTO draft (workspace_id, path, typ, value, email) VALUES

backend/windmill-api/src/drafts.rs

@@ -773,9 +773,14 @@ async fn move_draft(
     if moved.is_none() {
         let row = sqlx::query!(
             r#"SELECT
+                 -- Own row first: with both an own and a legacy row at the destination,
+                 -- a bare LIMIT 1 would name an arbitrary one and the two need different
+                 -- remedies (discard your own vs. ask an admin).
                  (SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3
                   AND typ::text = ANY($6::text[]) AND (email = $4 OR email IS NULL)
-                  LIMIT 1) as "at_target",
+                  ORDER BY email NULLS LAST LIMIT 1) as "at_target",
+                 EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3
+                        AND typ::text = ANY($6::text[]) AND email = $4) as "at_target_own!",
                  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
@@ -811,7 +816,17 @@ async fn move_draft(
             // Naming the kind matters for the app pair: a classic-app draft refusing a
             // raw-app move is invisible in the raw-app list the caller is looking at.
             let occupant = occupant.replace('_', " ");
-            format!("You already have a draft at '{new_path}' ({occupant})")
+            if row.at_target_own {
+                format!("You already have a draft at '{new_path}' ({occupant})")
+            } else {
+                // An ownerless row the caller cannot clear themselves, so send them to
+                // the one place it can be resolved rather than to "discard your draft".
+                format!(
+                    "A legacy workspace draft with no owner is already at '{new_path}' \
+                     ({occupant}). A workspace admin can claim or discard it on the Review & \
+                     deploy drafts page."
+                )
+            }
         } else {
             format!("You have no draft at '{path}'")
         }));

frontend/src/lib/components/DiffDrawer.svelte

@@ -78,6 +78,26 @@
 		diffViewer?.closeDrawer()
 	}
 
+	/** Counted per opening, and counted here rather than in the editor that opens one: a
+	 *  path change remounts the editor while this drawer stays mounted, so a counter local
+	 *  to the editor is one an outlived request still matches — it would open and fill the
+	 *  drawer with the item the user just left. Every write an opening makes (the blanking
+	 *  `openDrawer` included) checks `ownsOpening` first. */
+	let openingToken = 0
+
+	export function beginOpening(): number {
+		return ++openingToken
+	}
+
+	export function ownsOpening(token: number): boolean {
+		return token === openingToken
+	}
+
+	/** Drop the opening in flight: the editor that started it is going away. */
+	export function abandonOpening() {
+		openingToken++
+	}
+
 	function prepareDiff(data: Value) {
 		const metadata = structuredClone(cleanValueProperties(replaceFalseWithUndefined(data)))
 		const content = metadata['content']

frontend/src/lib/components/FlowBuilder.svelte

@@ -41,7 +41,7 @@
 	import DeployOverrideConfirmationModal from '$lib/components/common/confirmationModal/DeployOverrideConfirmationModal.svelte'
 	import AIChangesWarningModal from '$lib/components/copilot/chat/flow/AIChangesWarningModal.svelte'
 
-	import { getContext, setContext, untrack } from 'svelte'
+	import { getContext, onDestroy, setContext, untrack } from 'svelte'
 	import { writable } from 'svelte/store'
 	import CenteredPage from './CenteredPage.svelte'
 	import { Button } from './common'
@@ -484,7 +484,7 @@
 		})
 		// 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
+		if (opening != null && !diffDrawer?.ownsOpening(opening)) return
 		deployedValue = replaceFalseWithUndefined({
 			...flow,
 			edited_at: undefined,
@@ -1166,22 +1166,26 @@
 		}
 	}
 
-	/** 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
+	// An opening outlives this editor when a path change remounts it mid-fetch; without
+	// this it would still open the drawer on the item the user left.
+	onDestroy(() => diffDrawer?.abandonOpening())
 
 	export async function openDiffDrawer() {
-		const opening = ++diffOpening
 		if (!savedFlow) return
+		// The fetches below are awaited, so a reopen (or a path change, which remounts
+		// this editor but not the drawer) while they run must not have the older one
+		// land last. The drawer counts the openings for that reason.
+		const opening = diffDrawer?.beginOpening()
+		if (opening == null) return
 		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()
+		if (!diffDrawer?.ownsOpening(opening)) return
+		diffDrawer.openDrawer()
 		const currentFlow = flowStore.val
 		const versions = await deployedVersionOptions()
-		if (opening !== diffOpening) return
-		diffDrawer?.setDiff({
+		if (!diffDrawer?.ownsOpening(opening)) return
+		diffDrawer.setDiff({
 			mode: 'normal',
 			deployed: deployedValue ?? savedFlow,
 			deployedLabel,

frontend/src/lib/components/ScriptBuilder.svelte

@@ -87,7 +87,7 @@
 	import { writable } from 'svelte/store'
 	import { defaultScriptLanguages, processLangs } from '$lib/scripts'
 	import DefaultScripts from './DefaultScripts.svelte'
-	import { getContext, onMount, setContext, tick, untrack } from 'svelte'
+	import { getContext, onDestroy, onMount, setContext, tick, untrack } from 'svelte'
 	import EditorHeader from './EditorHeader.svelte'
 	import ScriptSettingsBadges from './ScriptSettingsBadges.svelte'
 	import Badge from './common/badge/Badge.svelte'
@@ -629,7 +629,7 @@
 
 		// 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
+		if (opening != null && !diffDrawer?.ownsOpening(opening)) return
 		deployedValue = replaceFalseWithUndefined({
 			...latestScript,
 			workspace_id: undefined,
@@ -873,15 +873,19 @@
 		}
 	}
 
-	/** 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
+	// An opening outlives this editor when a path change remounts it mid-fetch; without
+	// this it would still open the drawer on the item the user left.
+	onDestroy(() => diffDrawer?.abandonOpening())
 
 	export async function openDiffDrawer() {
-		const opening = ++diffOpening
 		if (!savedScript) {
 			return
 		}
+		// The fetches below are awaited, so a reopen (or a path change, which remounts
+		// this editor but not the drawer) while they run must not have the older one
+		// land last. The drawer counts the openings for that reason.
+		const opening = diffDrawer?.beginOpening()
+		if (opening == null) return
 		await syncWithDeployed(opening)
 
 		const currentDraftTriggers = structuredClone(triggersState.getDraftTriggersSnapshot())
@@ -900,12 +904,12 @@
 		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()
+		if (!diffDrawer?.ownsOpening(opening)) return
+		diffDrawer.openDrawer()
 		const headHash = (deployed as { hash?: string } | undefined)?.hash
 		const versions = await deployedVersionOptions(headHash)
-		if (opening !== diffOpening) return
-		diffDrawer?.setDiff({
+		if (!diffDrawer?.ownsOpening(opening)) return
+		diffDrawer.setDiff({
 			mode: 'normal',
 			deployed,
 			deployedLabel: deployedVersionLabel(deployed),

frontend/src/lib/components/diff_drawer.ts

@@ -56,4 +56,11 @@ export interface DiffDrawerI {
 	openDrawer: () => void
 	closeDrawer: () => void
 	setDiff: (diff: DiffDrawerDiff) => void
+	/** Claim the drawer for one opening. Filling it takes awaited fetches, and a path
+	 *  change remounts the editor while this drawer stays, so the token lives here: an
+	 *  editor checks `ownsOpening` before every write and drops the opening it started
+	 *  when it goes away. */
+	beginOpening: () => number
+	ownsOpening: (token: number) => boolean
+	abandonOpening: () => void
 }

frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte

@@ -28,7 +28,7 @@
 		Undo,
 		WandSparkles
 	} from 'lucide-svelte'
-	import { untrack } from 'svelte'
+	import { onDestroy, untrack } from 'svelte'
 	import { orderedJsonStringify, type Value, replaceFalseWithUndefined } from '../../utils'
 	import { random_adj } from '$lib/components/random_positive_adjetive'
 
@@ -406,7 +406,7 @@
 
 		// 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
+		if (opening != null && !diffDrawer?.ownsOpening(opening)) return
 		deployedBy = deployedApp.created_by
 		const shownVersions = (deployedApp as { versions?: number[] }).versions
 		deployedVersionShown = Array.isArray(shownVersions)
@@ -453,25 +453,29 @@
 		}
 	}
 
-	/** 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
+	// An opening outlives this editor when a path change remounts it mid-fetch; without
+	// this it would still open the drawer on the item the user left.
+	onDestroy(() => diffDrawer?.abandonOpening())
 
 	export async function openDiffDrawer() {
-		const opening = ++diffOpening
 		if (!savedApp) {
 			return
 		}
+		// The fetches below are awaited, so a reopen (or a path change, which remounts
+		// this editor but not the drawer) while they run must not have the older one
+		// land last. The drawer counts the openings for that reason.
+		const opening = diffDrawer?.beginOpening()
+		if (opening == null) return
 
 		// deployedValue should be syncronized when we open Diff
 		await syncWithDeployed(opening)
 
 		// Blanking the drawer belongs to the opening that will fill it.
-		if (opening !== diffOpening) return
-		diffDrawer?.openDrawer()
+		if (!diffDrawer?.ownsOpening(opening)) return
+		diffDrawer.openDrawer()
 		const versions = await deployedVersionOptions()
-		if (opening !== diffOpening) return
-		diffDrawer?.setDiff({
+		if (!diffDrawer?.ownsOpening(opening)) return
+		diffDrawer.setDiff({
 			mode: 'normal',
 			deployed: deployedValue ?? stripRawAppDiffNoise(savedApp),
 			versions,
@@ -692,12 +696,16 @@
 						if (!savedApp || newApp) {
 							return
 						}
+						// The other entry point into the same drawer, so it takes an opening too.
+						const opening = diffDrawer?.beginOpening()
+						if (opening == null) return
 						// deployedValue should be syncronized when we open Diff
-						await syncWithDeployed()
+						await syncWithDeployed(opening)
 
+						if (!diffDrawer?.ownsOpening(opening)) return
 						saveDrawerOpen = false
-						diffDrawer?.openDrawer()
-						diffDrawer?.setDiff({
+						diffDrawer.openDrawer()
+						diffDrawer.setDiff({
 							mode: 'normal',
 							deployed: deployedValue ?? stripRawAppDiffNoise(savedApp),
 							current: currentDiffValue,

frontend/src/routes/(root)/(logged)/scripts/edit/[...path]/+page.svelte

@@ -384,15 +384,19 @@
 				OtherUserDraftLoad.clear($workspaceStore!, 'script', draftPath)
 			}
 			if (pendingLoad) {
+				const theirs = (pendingLoad.value as { parent_hash?: string })?.parent_hash
 				const loadedValue = {
 					...deployedScript,
 					...(pendingLoad.value as object),
-					parent_hash: (pendingLoad.value as { parent_hash?: string })?.parent_hash ?? parentHash
+					parent_hash: theirs
 				} 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
+				// forked from. A draft that has none keeps none — seeding `parentHash`
+				// (the head, when we have no draft here) would mark their older content as
+				// forked from the current deploy and silence the stale prompt.
+				if (theirs == null) delete (loadedValue as { parent_hash?: string }).parent_hash
+				draftBaseHash = theirs
 				if (hasOwnDraft) {
 					OtherUserDraftLoad.beginOverlay({
 						workspace: $workspaceStore!,