fix(editors): read what a save sends before it is sent, and settle its draft in one place

The resource save read its snapshot back out of the live form after the request
returned, so the "did anything change meanwhile" test compared the edit against
itself and always said no — the mid-request edit became the deployed baseline
instead. Its payloads, its submitted path and the schedule's config are now all
pinned before the first await, which also makes the schedule's test see edits to
`args`, `retry` and `labels` rather than only its scalar fields.

The three copies of the settling rule become `settleDraftAfterWrite`. A save
that moved the item resets the cell rather than keeping a diverged one: the edit
cannot follow — a freshly acquired cell reads only its own default — and the
alternative is a draft stranded under a path the item no longer occupies.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KcyBRxE8j3ujCYkum9KrC4
This commit is contained in:
Diego Imbert
2026-09-06 04:07:20 +02:00
co-authored by Claude Opus 5
parent 0635901e7f
commit 67c1faed53
6 changed files with 158 additions and 40 deletions
@@ -12,7 +12,12 @@
import { resource } from 'runed'
import { getUserExt } from '$lib/user'
import type { UserExt } from '$lib/stores'
import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte'
import {
UserDraft,
draftValuesEqual,
settleDraftAfterWrite,
type UserDraftHandle
} from '$lib/userDraft.svelte'
import { setLocalDraftHint } from '$lib/localDraftHints.svelte'
interface Props {
@@ -331,12 +336,21 @@
/** Whether the write landed. It toasts its own failure, so most callers ignore this;
* one that follows the save with bookkeeping of its own has to know not to. */
export async function save(): Promise<boolean> {
const dirty = dirtyWorkspaces
// Everything the writes send, read before the first await. The form stays
// editable while they are in flight, so read later these would be whatever
// the user has since typed — sent under an earlier workspace's path, and
// adopted as a baseline the server never saw.
const from = initialPath ?? ''
const payloads = dirtyWorkspaces.map((ws) => ({
ws,
s: $state.snapshot(states[ws].draft!) as ResourceState,
ini: $state.snapshot(initialStates[ws]) as ResourceState,
existed: !!existedInitially[ws]
}))
const savedPath = payloads[0]?.s.path ?? from
try {
for (const ws of dirty) {
const s = states[ws].draft!
const ini = initialStates[ws]
if (existedInitially[ws]) {
for (const { ws, s, ini, existed } of payloads) {
if (existed) {
await ResourceService.updateResource({
workspace: ws,
path: ini.path,
@@ -366,22 +380,20 @@
})
}
// Reset the handle to the new deployed baseline via `discard`, not
// `remove`. See VariableEditor for the full rationale — including why the
// cell is only dropped while it still holds what was written.
const written = $state.snapshot(s) as ResourceState
initialStates[ws] = written
// `remove`. See VariableEditor for the full rationale.
initialStates[ws] = s
existedInitially[ws] = true
if (draftValuesEqual(states[ws]?.draft, written)) {
UserDraft.discard('resource', initialPath ?? '', written, { workspace: ws })
}
settleDraftAfterWrite('resource', s, states[ws]?.draft, from, savedPath, { workspace: ws })
// Path now exists server-side — drop the autocomplete cache so
// it shows up immediately instead of after the 60s TTL.
invalidateWorkspacePaths(ws)
}
sendUserToast(
dirty.length > 1 ? `Saved resource in ${dirty.length} workspaces` : `Saved resource`
payloads.length > 1
? `Saved resource in ${payloads.length} workspaces`
: `Saved resource`
)
dispatch('refresh', current?.path ?? path)
dispatch('refresh', savedPath)
return true
} catch (err) {
sendUserToast(`Could not save resource: ${err.body ?? err.message}`, true)
@@ -218,6 +218,10 @@
// workspace, while the write is in flight.
const from = path
const fromWs = effectiveWorkspace
// The path the form holds now is the one `save()` is about to send; read
// after the await it would be a rename the user typed meanwhile, and the
// tab would follow to a path this write never created.
const submitted = livePath ?? path
// Closed before the write is awaited, the way it always was: `save()` toasts its
// own failures and never rejects, so waiting would only add visible lag to every
// caller of this drawer. `onSaved` still fires after the write lands.
@@ -234,7 +238,7 @@
// Rendered inline there is no drawer to close, so the mounted editor would
// otherwise keep the pre-save baseline. Follow a rename before remounting,
// or it comes back up on a path the save just moved the item off.
const savedPath = livePath ?? path
const savedPath = submitted
if (savedPath) path = savedPath
editorGeneration++
onSaved?.(savedPath, from, fromWs)
@@ -22,7 +22,12 @@
import { resource } from 'runed'
import { getUserExt } from '$lib/user'
import type { UserExt } from '$lib/stores'
import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte'
import {
UserDraft,
draftValuesEqual,
settleDraftAfterWrite,
type UserDraftHandle
} from '$lib/userDraft.svelte'
import LocalDraftBanner from './LocalDraftBanner.svelte'
import { isEncryptedDraftValue } from '$lib/encryptedDraft'
import { setLocalDraftHint } from '$lib/localDraftHints.svelte'
@@ -329,15 +334,13 @@
initialStates[ws] = s
existedInitially[ws] = true
}
// The just-saved state is the new deployed baseline; reset the
// handle to it via `discard` (not `remove` — blanking the cell to
// `undefined` reads as dirty). The `value: null` POST also deletes
// the server draft row so `is_draft` clears on refetch. Only while the
// cell still holds what was written: the form stays editable during the
// request, and an edit made then is a change on top of the save.
if (draftValuesEqual(states[ws]?.draft, s)) {
UserDraft.discard('variable', from ?? '', s, { workspace: ws })
}
// The just-saved state is the new deployed baseline; `settleDraftAfterWrite`
// resets the handle to it via `discard` (not `remove` — blanking the cell
// to `undefined` reads as dirty), and carries a mid-request edit onto the
// path the save wrote to.
settleDraftAfterWrite('variable', s, states[ws]?.draft, from ?? '', savedPath ?? '', {
workspace: ws
})
// Path now exists server-side — drop the autocomplete cache so
// it shows up immediately instead of after the 60s TTL.
invalidateWorkspacePaths(ws)
@@ -44,8 +44,7 @@
import { runScheduleNow } from '../scheduled/utils'
import { handleConfigChange } from '../utils'
import { withForkConflictRetry } from '$lib/utils/forkConflict'
import { deepEqual } from 'fast-equals'
import { normalizeDraftForCompare } from '$lib/userDraft.svelte'
import { settleDraftAfterWrite } from '$lib/userDraft.svelte'
import { useTriggerDraftSync } from '../useTriggerDraftSync.svelte'
import LocalDraftBanner from '$lib/components/LocalDraftBanner.svelte'
import TextInput from '$lib/components/text_input/TextInput.svelte'
@@ -618,21 +617,21 @@
async function scheduleScript(): Promise<void> {
const previousPath = initialPath
const previousWs = wsId
const scheduleCfg = getScheduleCfg()
// Snapshotted, not just built: the object is fresh but its `args`, `retry` and
// `labels` alias live state, so an edit made while the request is in flight
// would read back as part of what was sent.
const scheduleCfg = $state.snapshot(getScheduleCfg()) as Record<string, any>
deploymentLoading = true
const isSaved = await saveScheduleFromCfg(scheduleCfg, edit, wsId!)
if (isSaved) {
// Drop the local draft only while the form still holds what was saved: it
// stays editable during the request, and an edit made then is a change on
// top of the save, which the reset would silently swallow.
if (
deepEqual(
normalizeDraftForCompare($state.snapshot(getScheduleCfg())),
normalizeDraftForCompare(scheduleCfg)
)
) {
draftSync.discard(previousPath, scheduleCfg)
}
settleDraftAfterWrite(
'trigger_schedule',
scheduleCfg,
getScheduleCfg(),
previousPath,
scheduleCfg.path,
{ workspace: previousWs ?? undefined }
)
onUpdate?.(scheduleCfg.path, previousPath, previousWs)
drawer?.closeDrawer()
}
+22
View File
@@ -268,6 +268,28 @@ export function draftValuesEqual(a: unknown, b: unknown): boolean {
return deepEqual(normalizeDraftForCompare(a), normalizeDraftForCompare(b))
}
/**
* Settle a draft cell after the write of `written` — the value actually sent —
* landed. A form stays editable while its request is in flight, so `live` may
* hold a newer edit; that is a change on top of the save rather than part of it,
* and resetting the cell would swallow it silently. A save that also moved the
* item is the exception: the edit cannot follow (a freshly acquired cell reads
* only its own default), so the cell is reset rather than left orphaned under a
* path the item no longer occupies.
*/
export function settleDraftAfterWrite<V>(
itemKind: UserDraftItemKind,
written: V,
live: V | undefined,
fromPath: string,
savedPath: string,
opts?: UserDraftOptions
): void {
if (savedPath !== fromPath || draftValuesEqual(live, written)) {
UserDraft.discard(itemKind, fromPath, written, opts)
}
}
export type UserDraftHandle<V> = {
get draft(): V | undefined
set draft(value: V | undefined)
@@ -0,0 +1,78 @@
import { describe, it, expect, beforeEach, vi } from 'vitest'
const discard = vi.fn()
vi.mock('./gen', () => ({ DraftService: { updateDraft: vi.fn() } }))
vi.mock('./gen/core/OpenAPI', () => ({ OpenAPI: { BASE: '' } }))
vi.mock('./localDraftHints.svelte', () => ({ setLocalDraftHint: vi.fn() }))
import { settleDraftAfterWrite, UserDraft } from './userDraft.svelte'
beforeEach(() => {
discard.mockClear()
vi.spyOn(UserDraft, 'discard').mockImplementation(discard as any)
})
const OPTS = { workspace: 'ws' }
const sent = { path: 'u/me/a', value: 'sent' }
/**
* The forms stay editable while their save is in flight, so the cell can hold a
* newer edit by the time the write returns. Resetting it unconditionally — what
* every editor did — swallows that edit with no trace.
*/
describe('settleDraftAfterWrite', () => {
it('resets the cell when it still holds what was written', () => {
settleDraftAfterWrite('variable', sent, { ...sent }, 'u/me/a', 'u/me/a', OPTS)
expect(discard).toHaveBeenCalledWith('variable', 'u/me/a', sent, OPTS)
})
it('keeps an edit made while the write was in flight', () => {
settleDraftAfterWrite(
'variable',
sent,
{ path: 'u/me/a', value: 'typed' },
'u/me/a',
'u/me/a',
OPTS
)
expect(discard).not.toHaveBeenCalled()
})
// A renamed save is the exception: a freshly acquired cell reads only its own
// default, so the edit cannot follow the item and the alternative to resetting
// is an orphan draft under a path the item no longer occupies.
it('resets even a diverged cell when the write moved the item', () => {
settleDraftAfterWrite(
'variable',
sent,
{ path: 'u/me/a', value: 'typed' },
'u/me/a',
'u/me/b',
OPTS
)
expect(discard).toHaveBeenCalledWith('variable', 'u/me/a', sent, OPTS)
})
it('compares through the draft normalization, so a nested edit is not missed', () => {
const written = { path: 'u/me/a', args: { host: 'h' } }
settleDraftAfterWrite(
'resource',
written,
{ path: 'u/me/a', args: { host: 'h' } },
'u/me/a',
'u/me/a',
OPTS
)
expect(discard).toHaveBeenCalledTimes(1)
discard.mockClear()
settleDraftAfterWrite(
'resource',
written,
{ path: 'u/me/a', args: { host: 'edited' } },
'u/me/a',
'u/me/a',
OPTS
)
expect(discard).not.toHaveBeenCalled()
})
})