mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-24 00:00:46 +00:00
fix(sessions): bypass UserDraft inside session panes + sessionUnread crash
After merging main's UserDraft PR (#9121) into the sessions branch, two integration issues surfaced: 1. AppEditor.svelte calls `UserDraft.use<App>('app', path)` at the component level — keyed by ($workspaceStore, 'app', path). Sessions that haven't materialized a fork yet stay at the user's main workspace, so a session targeting an app at the same path as a regular /apps/edit tab shared the same LS key. The session would read the regular tab's autosave and write its fork-edits back over it. Gate UserDraft.use on `!getContext('aiChatManager')` — sessions inject the manager via setContext, so inside a session pane the handle is `undefined`, stateApp falls through to the `app` prop the session loaded, and the auto-save $effect bails. Same gate on the four UserDraft.remove call sites in AppEditorHeader and RawAppEditorHeader so save/deploy from a session pane doesn't wipe the LS draft of a non-session tab at the same path. 2. sessionUnread.svelte.ts called useLocalStorageValue at module scope. Main's PR added a deep-mutation $effect inside that helper, which now requires component-initialization context — every page crashed at import time with `Svelte error: effect_orphan`. Replaced with a plain module-level $state + manual localStorage persist; same reactivity contract for callers. 3. ScriptEditorView.svelte was passing a `replaceStateFn` prop that ScriptBuilder dropped on main. Removed. Verified end-to-end with Playwright: - /flows/edit/{path} regression: UserDraft handle still created, no console errors - /sessions loads, sessionUnread doesn't crash - Session targeting non-raw app `u/admin/userdraft_collision_test` displays the fork content (FORK_ONLY_MARKER) even with an LS poison at `userdraft/w/local/app/{path}` containing a POISONED_BY_REGULAR_TAB_AUTOSAVE marker; poison remains untouched after the session loads and renders Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
|
||||
const bubble = createBubbler()
|
||||
import SplitPanesWrapper from '$lib/components/splitPanes/SplitPanesWrapper.svelte'
|
||||
import { onMount, setContext, untrack } from 'svelte'
|
||||
import { getContext, onMount, setContext, untrack } from 'svelte'
|
||||
import { twMerge } from 'tailwind-merge'
|
||||
|
||||
import { Pane, Splitpanes } from 'svelte-splitpanes'
|
||||
@@ -85,15 +85,23 @@
|
||||
|
||||
migrateApp(untrack(() => app))
|
||||
|
||||
// Inside a session pane the AIChatManager is injected via context. Sessions
|
||||
// have their own state machinery (sessionRuntime + per-fork backend), and
|
||||
// the user-facing $workspaceStore stays on the main workspace even when
|
||||
// the session is editing in a fork — so a UserDraft handle here would
|
||||
// share its LS key with the regular /apps/edit route and clobber both
|
||||
// sides' autosaves. Skip UserDraft entirely in that case.
|
||||
const inSessionPane = !!getContext('aiChatManager')
|
||||
|
||||
const appDraftPath = newApp ? '' : (path ?? '')
|
||||
const appDraftHandle = UserDraft.use<App>('app', appDraftPath)
|
||||
const appDraftHandle = inSessionPane ? undefined : UserDraft.use<App>('app', appDraftPath)
|
||||
// Prefer the persisted autosave over the prop when both exist (e.g.
|
||||
// /apps/add reload: the route always initializes `app` to an empty
|
||||
// template, but the user's last session is sitting in LS under the
|
||||
// empty-path entry). The route is responsible for wiping the entry
|
||||
// (`UserDraft.remove`) when it wants to force a fresh start —
|
||||
// `?nodraft=true`, template/hub loads, etc.
|
||||
const stateApp = $state(untrack(() => appDraftHandle.draft ?? app))
|
||||
const stateApp = $state(untrack(() => appDraftHandle?.draft ?? app))
|
||||
const appStore = writable<App>(stateApp)
|
||||
// Captured once on mount: the load-time revs are only used as the
|
||||
// seed meta on the very first persist of this entry. After that the
|
||||
@@ -113,6 +121,7 @@
|
||||
let firstMirror = true
|
||||
$effect(() => {
|
||||
readFieldsRecursively(stateApp)
|
||||
if (!appDraftHandle) return
|
||||
untrack(() => {
|
||||
// Resolve the meta to attach BEFORE the wipe — the wipe clears
|
||||
// in-memory meta and would otherwise force-seed `initialRevs`
|
||||
|
||||
@@ -172,6 +172,11 @@
|
||||
const { history, jobsDrawerOpen, refreshComponents } =
|
||||
getContext<AppEditorContext>('AppEditorContext')
|
||||
|
||||
// Sessions inject an AIChatManager via context; AppEditor skips its
|
||||
// UserDraft handle in that case, so the cleanup calls here must skip too
|
||||
// (otherwise we'd wipe a non-session tab's autosave at the same path).
|
||||
const inSessionPane = !!getContext('aiChatManager')
|
||||
|
||||
const loading = $state({
|
||||
publish: false,
|
||||
save: false,
|
||||
@@ -231,7 +236,7 @@
|
||||
}
|
||||
closeSaveDrawer()
|
||||
sendUserToast('App deployed successfully')
|
||||
UserDraft.remove('app', path)
|
||||
if (!inSessionPane) UserDraft.remove('app', path)
|
||||
onSavedNewAppPath?.(path)
|
||||
} catch (e) {
|
||||
sendUserToast('Error creating app', e)
|
||||
@@ -331,7 +336,7 @@
|
||||
|
||||
closeSaveDrawer()
|
||||
sendUserToast('App deployed successfully')
|
||||
UserDraft.remove('app', $appPath)
|
||||
if (!inSessionPane) UserDraft.remove('app', $appPath)
|
||||
if ($appPath !== npath) {
|
||||
onSavedNewAppPath?.(npath)
|
||||
}
|
||||
@@ -407,7 +412,7 @@
|
||||
// The initial draft was promoted to a real path on the backend —
|
||||
// drop the autosave keyed on the prior (possibly empty) path so
|
||||
// a future "+ App" click opens on a clean slate.
|
||||
UserDraft.remove('app', $appPath)
|
||||
if (!inSessionPane) UserDraft.remove('app', $appPath)
|
||||
onSavedNewAppPath?.(newEditedPath)
|
||||
} catch (e) {
|
||||
sendUserToast('Error saving initial draft', e)
|
||||
@@ -498,7 +503,7 @@
|
||||
}
|
||||
|
||||
sendUserToast('Draft saved')
|
||||
UserDraft.remove('app', path)
|
||||
if (!inSessionPane) UserDraft.remove('app', path)
|
||||
loading.saveDraft = false
|
||||
if (newApp || savedApp.draft_only) {
|
||||
onSavedNewAppPath?.(newEditedPath || path)
|
||||
|
||||
@@ -269,7 +269,7 @@
|
||||
}
|
||||
closeSaveDrawer()
|
||||
sendUserToast('App deployed successfully')
|
||||
UserDraft.remove('raw_app', path)
|
||||
if (!inSessionPane) UserDraft.remove('raw_app', path)
|
||||
dispatch('savedNewAppPath', path)
|
||||
} catch (e) {
|
||||
sendUserToast(`Error creating app: ${e.body ?? e.message}`, true)
|
||||
@@ -380,7 +380,7 @@
|
||||
|
||||
closeSaveDrawer()
|
||||
sendUserToast('App deployed successfully')
|
||||
UserDraft.remove('raw_app', appPath)
|
||||
if (!inSessionPane) UserDraft.remove('raw_app', appPath)
|
||||
if (appPath !== npath) {
|
||||
dispatch('savedNewAppPath', npath)
|
||||
}
|
||||
@@ -462,7 +462,7 @@
|
||||
// The initial draft was promoted to a real path on the backend —
|
||||
// drop the autosave keyed on the prior (possibly empty) path so
|
||||
// a future "+ App" click opens on a clean slate.
|
||||
UserDraft.remove('raw_app', appPath)
|
||||
if (!inSessionPane) UserDraft.remove('raw_app', appPath)
|
||||
dispatch('savedNewAppPath', newEditedPath)
|
||||
} catch (e) {
|
||||
sendUserToast(`Error saving initial draft: ${e.body ?? e.message}`, true)
|
||||
@@ -563,7 +563,7 @@
|
||||
}
|
||||
|
||||
sendUserToast('Draft saved')
|
||||
UserDraft.remove('raw_app', path)
|
||||
if (!inSessionPane) UserDraft.remove('raw_app', path)
|
||||
loading.saveDraft = false
|
||||
if (newApp || savedApp.draft_only) {
|
||||
dispatch('savedNewAppPath', newEditedPath || path)
|
||||
|
||||
@@ -112,7 +112,6 @@
|
||||
initialPath={path}
|
||||
fullyLoaded={!runtime.loadingScript}
|
||||
disableHistoryChange={true}
|
||||
replaceStateFn={() => {}}
|
||||
{diffDrawer}
|
||||
{onNavigate}
|
||||
{initialTestPanelCollapsed}
|
||||
|
||||
@@ -1,40 +1,60 @@
|
||||
import { useLocalStorageValue } from '$lib/svelte5Utils.svelte'
|
||||
import type { SessionRuntime } from './sessionRuntime.svelte'
|
||||
|
||||
// Per-user, per-session "last seen" marker — count of displayMessages the
|
||||
// last time the user was actually on that session's page. Compared against
|
||||
// the runtime's current message count to derive an unread badge.
|
||||
//
|
||||
// Stored as a single localStorage entry holding Record<sessionId, count>
|
||||
// to avoid scattering keys across the namespace and to make
|
||||
// useLocalStorageValue's reactivity cover all sessions at once.
|
||||
const lastSeenStore = useLocalStorageValue<Record<string, number>>(
|
||||
'windmill_sessions_last_seen_counts',
|
||||
{}
|
||||
)
|
||||
// Stored as a single localStorage entry holding Record<sessionId, count>.
|
||||
// Module-level $state for cross-session reactivity; can't use
|
||||
// `useLocalStorageValue` here because its internal $effect requires a
|
||||
// component-initialization context (we run at import time).
|
||||
const LS_KEY = 'windmill_sessions_last_seen_counts'
|
||||
|
||||
function readInitial(): Record<string, number> {
|
||||
if (typeof window === 'undefined') return {}
|
||||
try {
|
||||
const raw = localStorage.getItem(LS_KEY)
|
||||
return raw ? JSON.parse(raw) : {}
|
||||
} catch {
|
||||
return {}
|
||||
}
|
||||
}
|
||||
|
||||
const lastSeen = $state<{ val: Record<string, number> }>({ val: readInitial() })
|
||||
|
||||
function persist(): void {
|
||||
if (typeof window === 'undefined') return
|
||||
try {
|
||||
localStorage.setItem(LS_KEY, JSON.stringify(lastSeen.val))
|
||||
} catch (e) {
|
||||
console.error('sessionUnread: localStorage write failed', e)
|
||||
}
|
||||
}
|
||||
|
||||
// Mark the session as seen up to `count` messages. No-op when already at
|
||||
// or past that count (idempotent — call freely from $effects).
|
||||
export function markSessionSeen(sessionId: string, count: number) {
|
||||
const current = lastSeenStore.val[sessionId] ?? 0
|
||||
const current = lastSeen.val[sessionId] ?? 0
|
||||
if (current >= count) return
|
||||
lastSeenStore.val = { ...lastSeenStore.val, [sessionId]: count }
|
||||
lastSeen.val = { ...lastSeen.val, [sessionId]: count }
|
||||
persist()
|
||||
}
|
||||
|
||||
// Drop the session entry entirely (used on delete so we don't leak
|
||||
// stale ids into localStorage indefinitely).
|
||||
export function forgetSessionSeen(sessionId: string) {
|
||||
if (!(sessionId in lastSeenStore.val)) return
|
||||
const next = { ...lastSeenStore.val }
|
||||
if (!(sessionId in lastSeen.val)) return
|
||||
const next = { ...lastSeen.val }
|
||||
delete next[sessionId]
|
||||
lastSeenStore.val = next
|
||||
lastSeen.val = next
|
||||
persist()
|
||||
}
|
||||
|
||||
// Number of unread messages for a session. Undefined / unloaded runtime
|
||||
// returns 0 — until messages are hydrated we don't know what's new.
|
||||
export function unreadCountFor(sessionId: string, runtime: SessionRuntime | undefined): number {
|
||||
if (!runtime) return 0
|
||||
const seen = lastSeenStore.val[sessionId] ?? 0
|
||||
const seen = lastSeen.val[sessionId] ?? 0
|
||||
const total = runtime.manager.displayMessages.length
|
||||
return Math.max(0, total - seen)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user