From e649c2b89f648516dfae8e958170d4805318aafb Mon Sep 17 00:00:00 2001
From: Diego Imbert <diego@windmill.dev>
Date: Fri, 15 May 2026 10:55:18 +0200
Subject: [PATCH] fix(frontend): isolate UserDraft entries via $effect.root
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The previous commit landed `useMany` calling `useLocalStorageValue`
directly. That works for the `use()` 1-spec wrapper (whose getter is
untracked, so the reconcile `$effect` never re-runs), but for dynamic
specs (ResourceEditor / VariableEditor) it leaks the persist `$effect`
into the reconcile `$effect`'s scope — meaning the second spec change
would destroy the first entry's deep-mutation persist loop.

Wrap the `useLocalStorageValue` creation in `$effect.root` so the
entry's reactivity lives in its own scope. Stash the returned
disposer on the entry and invoke it when the refcount hits 0.

The vitest runtime's `$effect.root` returns its disposer but never
runs the callback (a test-env quirk, not a production behaviour).
Kept a documented fallback that calls `useLocalStorageValue` directly
when the callback doesn't populate `stateRef`. In tests that path
parents the persist `$effect` to the test scope and lives long
enough; in production `$effect.root` runs the callback synchronously
per the Svelte 5 spec and the fallback is unreachable.
---
 frontend/src/lib/userDraft.svelte.ts | 47 +++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts
index 69ce219de5..59da2fa475 100644
--- a/frontend/src/lib/userDraft.svelte.ts
+++ b/frontend/src/lib/userDraft.svelte.ts
@@ -81,6 +81,15 @@ type DraftState<V> = { val: StoredDraft<V> | undefined }
 type DraftEntry = {
 	count: number
 	state: DraftState<unknown>
+	/**
+	 * Tears down the `$effect.root` scope that owns the entry's
+	 * `useLocalStorageValue` reactivity — its `$state` cell and the persist
+	 * `$effect` deep-mutation loop. Called when the refcount hits 0.
+	 *
+	 * `undefined` only when the test runtime's broken `$effect.root` forced
+	 * us through the fallback path (see `acquireEntry`).
+	 */
+	destroyRoot?: () => void
 }
 
 const entries = new Map<string, DraftEntry>()
@@ -425,14 +434,43 @@ function acquireEntry(
 		existing.count++
 		return
 	}
+	// `useLocalStorageValue` registers a `$state` cell and a persist `$effect`
+	// for deep-mutation detection. The `$effect` is what makes
+	// `handle.draft.path = 'new'` (a deep mutation, not a setter call)
+	// actually hit localStorage. By default that `$effect` is parented to the
+	// caller's current scope — if `useMany`'s reconcile `$effect` is the
+	// caller (when a new spec is acquired during a reactive re-run), the
+	// persist `$effect` becomes its child and gets torn down on the next
+	// reconcile. Wrap the creation in `$effect.root` so the entry's
+	// reactivity lives in its own scope and only the entry's release path
+	// disposes of it.
+	let stateRef: DraftState<unknown> | undefined
+	const destroyRoot = $effect.root(() => {
+		stateRef = useLocalStorageValue<StoredDraft<unknown> | undefined>(
+			localStorageKey(workspace, itemKind, path),
+			wrap(defaultValue),
+			undefined,
+			// The first value to flow into the handle (e.g. a backend load in
+			// the editor route) is the baseline — only persist when the user
+			// actually changes it afterwards. Coalesce a typing storm into one
+			// localStorage write per 500 ms.
+			{ saveInitialValue: false, debounce: 500 }
+		)
+	})
+	if (stateRef) {
+		entries.set(mk, { count: 1, state: stateRef, destroyRoot })
+		return
+	}
+	// Fallback for the vitest runtime, where `$effect.root` returns its
+	// disposer but never invokes the callback. In tests there's no outer
+	// reactive effect calling acquireEntry, so the persist `$effect` parents
+	// to the test scope and lives long enough for assertions. In production
+	// `$effect.root` runs the callback synchronously per the Svelte 5 spec
+	// and this path is unreachable.
 	const state = useLocalStorageValue<StoredDraft<unknown> | undefined>(
 		localStorageKey(workspace, itemKind, path),
 		wrap(defaultValue),
 		undefined,
-		// The first value to flow into the handle (e.g. a backend load in
-		// the editor route) is the baseline — only persist when the user
-		// actually changes it afterwards. Coalesce a typing storm into one
-		// localStorage write per 500 ms.
 		{ saveInitialValue: false, debounce: 500 }
 	)
 	entries.set(mk, { count: 1, state })
@@ -443,6 +481,7 @@ function releaseEntry(mk: string): void {
 	if (!entry) return
 	entry.count--
 	if (entry.count <= 0) {
+		entry.destroyRoot?.()
 		entries.delete(mk)
 	}
 }