fix(sessions): serialize saves per draft cell, and let a deploy say where it landed

Two structural gaps, closed at the level they exist at rather than per call site.

An item is no longer owned by one editor: duplicate tabs and warm sessions mount
several over one (workspace, kind, path) cell, so a flag kept per editor
serializes nothing. The window each save already opens is per key, so that is
what says a write is in flight — every editor's Save reads it, and the local
flags are gone.

And a chat tool told the hosts only its own arguments, so anything it learned
while running was invisible to them. A deploy lands on the draft's own path
field, which a hosted editor moves by renaming a draft-only item, and the tab was
refreshed onto the path the item had left. The result now carries where it
landed, and a move is reported as the rename it is — the same report an editor's
own save makes.

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-07 18:36:06 +02:00
co-authored by Claude Opus 5
parent 822906e3cf
commit 1893132ccb
10 changed files with 188 additions and 76 deletions
@@ -17,6 +17,7 @@
draftValuesEqual,
flushDraftDelete,
beginDraftSettleWindow,
isDraftSaving,
settleDraftAfterWrite,
type UserDraftHandle
} from '$lib/userDraft.svelte'
@@ -277,8 +278,16 @@
}
})
// A save in flight for this resource, in this editor or another holding the same
// cell: inline there is no drawer to close over the button, and duplicate tabs
// and warm sessions mount several editors over one item. Two writes at once and
// the older one landing last overwrites the newer deployment.
const saveInFlight = $derived(
dirtyWorkspaces.some((ws) => isDraftSaving('resource', initialPath, { workspace: ws }))
)
$effect(() => {
canSave = anyDirty && dirtyValid && dirtyCanWrite
canSave = anyDirty && dirtyValid && dirtyCanWrite && !saveInFlight
})
// Drive the parent drawer's "unsaved changes" banner. The drawer chrome
@@ -368,10 +377,15 @@
// stay pointed at a path the item has moved off.
const written: { ws: string; path: string }[] = []
// A host left mid-write releases the cell each settle below reads; only inside
// this window is what it was holding remembered.
const closeSettleWindows = payloads.map(({ ws }) =>
beginDraftSettleWindow('resource', from, { workspace: ws })
)
// this window is what it was holding remembered. It is also what says a write
// is in flight for this cell, so a second editor on it cannot start one —
// checked before the first is opened, or this would see its own.
if (payloads.some(({ ws }) => isDraftSaving('resource', from, { workspace: ws }))) {
return { written: [], ok: false }
}
const closeSettleWindows = from
? payloads.map(({ ws }) => beginDraftSettleWindow('resource', from, { workspace: ws }))
: []
try {
for (const { ws, s, ini, existed } of payloads) {
if (existed) {
@@ -418,9 +432,7 @@
invalidateWorkspacePaths(ws)
}
sendUserToast(
payloads.length > 1
? `Saved resource in ${payloads.length} workspaces`
: `Saved resource`
payloads.length > 1 ? `Saved resource in ${payloads.length} workspaces` : `Saved resource`
)
dispatch('refresh', written.find((w) => w.ws === effectiveWorkspace)?.path ?? from)
return { written, ok: true }
@@ -61,10 +61,6 @@
// truth (see editorBody).
let editorGeneration = $state(0)
let canSave = $state(true)
// A save in flight. Inline there is no drawer to close over the button, so a
// second click would start an unserialized second write: the older snapshot
// lands last and overwrites the newer deployment.
let savePending = $state(false)
let resource_type: string | undefined = $state(undefined)
let defaultValues: Record<string, any> | undefined = $state(undefined)
@@ -227,47 +223,41 @@
unifiedSize="md"
startIcon={{ icon: Save }}
on:click={async () => {
if (savePending) return
savePending = true
try {
// Where this save started. Read before the await: an inline host can re-point
// the editor at another resource, or re-scope the session to another
// workspace, while the write is in flight.
const from = path
const fromWs = effectiveWorkspace
// The workspace whose version is on screen, which `WsSpecificVersions` can
// make a linked one: a rename made there is that workspace's alone, but the
// form is showing it, so the form follows it.
const shownWs = selected
// 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.
const saving = resourceEditor?.saveWritten()
drawer?.closeDrawer()
// What landed, per workspace, from the editor rather than the form: the form may
// be showing a linked workspace's variant, whose rename is not this host's.
const written = (await saving) ?? []
if (written.length === 0) return
const submitted =
written.find((w) => w.ws === shownWs)?.path ?? written.find((w) => w.ws === fromWs)?.path
// Follow a rename: rendered inline there is no drawer to close, so the mounted
// editor stays, and the key below remounts it on the path the item moved to.
// The editor adopts its own new baseline, so a same-path save needs nothing
// here — remounting it would re-read over an edit made during the write.
// Only while this is still the resource it saved: re-pointed mid-write,
// `path` and the mounted editor are another one's. And only a move: pointing a
// create at the path it just made re-keys the blank form's cell onto it, and
// the mirror then posts that blank state as the new item's draft.
if (path && path === from && submitted) path = submitted
// One report per workspace written, each carrying its own — a linked
// workspace's rename moves the tabs acting on it and no others. Reported
// even when this drawer has moved on: the write is a fact about the item.
for (const w of written) onSaved?.(w.path, from, w.ws)
} finally {
savePending = false
}
// Where this save started. Read before the await: an inline host can re-point
// the editor at another resource, or re-scope the session to another
// workspace, while the write is in flight.
const from = path
const fromWs = effectiveWorkspace
// The workspace whose version is on screen, which `WsSpecificVersions` can
// make a linked one: a rename made there is that workspace's alone, but the
// form is showing it, so the form follows it.
const shownWs = selected
// 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.
const saving = resourceEditor?.saveWritten()
drawer?.closeDrawer()
// What landed, per workspace, from the editor rather than the form: the form may
// be showing a linked workspace's variant, whose rename is not this host's.
const written = (await saving) ?? []
if (written.length === 0) return
const submitted =
written.find((w) => w.ws === shownWs)?.path ?? written.find((w) => w.ws === fromWs)?.path
// Follow a rename: rendered inline there is no drawer to close, so the mounted
// editor stays, and the key below remounts it on the path the item moved to.
// The editor adopts its own new baseline, so a same-path save needs nothing
// here — remounting it would re-read over an edit made during the write.
// Only while this is still the resource it saved: re-pointed mid-write,
// `path` and the mounted editor are another one's. And only a move: pointing a
// create at the path it just made re-keys the blank form's cell onto it, and
// the mirror then posts that blank state as the new item's draft.
if (path && path === from && submitted) path = submitted
// One report per workspace written, each carrying its own — a linked
// workspace's rename moves the tabs acting on it and no others. Reported
// even when this drawer has moved on: the write is a fact about the item.
for (const w of written) onSaved?.(w.path, from, w.ws)
}}
disabled={!canSave || savePending}
disabled={!canSave}
>
Save
</Button>
@@ -27,6 +27,7 @@
draftValuesEqual,
flushDraftDelete,
beginDraftSettleWindow,
isDraftSaving,
settleDraftAfterWrite,
type UserDraftHandle
} from '$lib/userDraft.svelte'
@@ -283,14 +284,15 @@
form?.setCode(getV.value ?? '')
}
// A save in flight. Inline there is no drawer to close over the button, so a
// second click would start an unserialized second write: the older snapshot
// lands last and overwrites the newer deployment.
let savePending = $state(false)
// A save in flight for this variable, in this editor or another holding the same
// cell: inline there is no drawer to close over the button, and duplicate tabs
// and warm sessions mount several editors over one item. Two writes at once and
// the older one landing last overwrites the newer deployment.
const saveInFlight = $derived(
dirtyWorkspaces.some((ws) => isDraftSaving('variable', editPath, { workspace: ws }))
)
async function save(): Promise<void> {
if (savePending) return
savePending = true
// Everything the writes need, read before the first await. An inline host can
// re-point this editor at another variable mid-flight, and every one of these
// would then be that variable's: the writes would send its state under this
@@ -320,10 +322,13 @@
// moves the tabs acting on it and no others.
const written: { ws: string; path: string }[] = []
// A host left mid-write releases the cell each settle below reads; only inside
// this window is what it was holding remembered.
const closeSettleWindows = payloads.map(({ ws }) =>
beginDraftSettleWindow('variable', from ?? '', { workspace: ws })
)
// this window is what it was holding remembered. It is also what says a write
// is in flight for this cell, so a second editor on it cannot start one —
// checked before the first is opened, or this would see its own.
if (payloads.some(({ ws }) => isDraftSaving('variable', from, { workspace: ws }))) return
const closeSettleWindows = from
? payloads.map(({ ws }) => beginDraftSettleWindow('variable', from, { workspace: ws }))
: []
try {
for (const { ws, s, ini, existed } of payloads) {
if (existed) {
@@ -411,7 +416,6 @@
}
} finally {
for (const close of closeSettleWindows) close()
savePending = false
}
}
</script>
@@ -456,7 +460,7 @@
{/if}
<Button
on:click={save}
disabled={savePending || !anyDirty || !dirtyValid || !dirtyCanWrite || pathError != ''}
disabled={saveInFlight || !anyDirty || !dirtyValid || !dirtyCanWrite || pathError != ''}
startIcon={{ icon: Save }}
variant="accent"
unifiedSize="sm"
@@ -7578,6 +7578,15 @@ async function deployDraft(
}
}
// Where the drawer kinds land is the draft's own path field, not the key it is
// stored under: a hosted editor renames a draft-only item by editing the config,
// which leaves the draft where it was. The script/flow/app branches resolve their
// own target above.
if (type !== 'script' && type !== 'flow' && type !== 'app') {
const draftPath = (draft.value as { path?: string } | undefined)?.path
if (draftPath) deployedPath = draftPath
}
// Deployed state moved for EVERY branch above (some bypass
// deployDraftToWorkspace, which invalidates on its own path) — evict cached
// fork comparisons before the fallible draft cleanup below.
@@ -7629,6 +7638,8 @@ async function deployDraft(
.join(' '),
type,
path,
// Where it landed, which a rename staged in the draft moves off `path`.
deployed_path: deployedPath,
triggerKind
},
null,
@@ -688,11 +688,11 @@ export function pendingUserActionDetail(
// the tool layer knowing about the UI. Single slot; the consumer filters by name
// and reads the tool args (e.g. the mutated item's `path`) to scope its refresh.
let toolCompletionListener:
| ((toolName: string, args: any, workspace: string) => void)
| ((toolName: string, args: any, workspace: string, result: string) => void)
| undefined
export function setToolCompletionListener(
fn: ((toolName: string, args: any, workspace: string) => void) | undefined
fn: ((toolName: string, args: any, workspace: string, result: string) => void) | undefined
): void {
toolCompletionListener = fn
}
@@ -722,8 +722,10 @@ async function callTool<T>({
}
const result = await tool.fn({ args, workspace, helpers, toolCallbacks, toolId })
// The workspace the tool acted on, so a consumer can tell a mutation in this
// session's workspace from the same path in another one.
toolCompletionListener?.(functionName, args, workspace)
// session's workspace from the same path in another one — and the result, which
// is where a tool says what it did that its args do not, such as the path a
// deploy landed on after a rename staged in the draft.
toolCompletionListener?.(functionName, args, workspace, result)
return result
}
@@ -17,6 +17,25 @@ describe('toolReloadEffect', () => {
expect(toolReloadEffect('create_folder', { name: 'f' }).pages).toEqual(['/folders'])
})
// A draft carries its own path field, so a hosted editor renames a draft-only
// item by editing the config and leaves the draft under the key it was stored
// at. The deploy lands on the config's path, and only the tool's own result
// says so — refreshing the args path would remount a tab on an empty one.
it('follows a deploy that landed on a renamed path', () => {
const effect = toolReloadEffect(
'deploy_workspace_item',
{ type: 'schedule', path: 'u/me/old' },
JSON.stringify({ success: true, path: 'u/me/old', deployed_path: 'u/me/new' })
)
expect(effect).toMatchObject({ path: 'u/me/old', to: 'u/me/new', entity: 'refresh' })
})
it('names no destination for an unreadable or absent result', () => {
const args = { type: 'schedule', path: 'u/me/s' }
expect(toolReloadEffect('deploy_workspace_item', args, 'not json').to).toBeUndefined()
expect(toolReloadEffect('deploy_workspace_item', args).to).toBeUndefined()
})
it('maps a trigger write to its kind-specific page', () => {
expect(toolReloadEffect('write_trigger', { kind: 'kafka' }).pages).toEqual(['/kafka_triggers'])
expect(toolReloadEffect('write_trigger', { kind: 'http' }).pages).toEqual(['/routes'])
@@ -29,10 +29,15 @@ export type ToolReloadEffect = {
* another path is not affected by it — unlike a list page, which shows every
* row and so reloads for any mutation on it. */
path?: string
/** Where it landed, when that is not `path`: a draft carries its own path
* field, so a hosted editor renames a draft-only item by editing the config
* and leaves the draft under the key it was stored at. A tab on `path` has to
* follow, exactly as it does for a rename the editor itself saved. */
to?: string
}
const NO_RELOAD: ToolReloadEffect = { pages: [], entity: 'none' }
export function toolReloadEffect(name: string, args: any): ToolReloadEffect {
export function toolReloadEffect(name: string, args: any, result?: string): ToolReloadEffect {
switch (name) {
// `path` rides along on the writes too: it is what scopes the refresh a write
// needs when its seed missed a still-loading editor (see effectForWrite).
@@ -52,8 +57,14 @@ export function toolReloadEffect(name: string, args: any): ToolReloadEffect {
// These all drop the draft the hosted editor is bound to: deploying or
// discarding replaces it with the deployed value, and deleting removes the
// item, so the editor is re-read from the server or left behind entirely.
case 'discard_local_draft':
case 'deploy_workspace_item':
return {
pages: pagesForItemType(args?.type, args),
entity: 'refresh',
path: itemPath(args),
to: deployedPath(result)
}
case 'discard_local_draft':
case 'rebase_draft':
return { pages: pagesForItemType(args?.type, args), entity: 'refresh', path: itemPath(args) }
case 'delete_workspace_item':
@@ -63,6 +74,19 @@ export function toolReloadEffect(name: string, args: any): ToolReloadEffect {
}
}
/** The path a deploy reported landing on. Read from the result rather than the
* args because only the tool knows it: the draft's own path field is what it
* deploys at. Anything unparseable leaves the mutation addressed by its args. */
function deployedPath(result: string | undefined): string | undefined {
if (!result) return undefined
try {
const p = (JSON.parse(result) as { deployed_path?: unknown }).deployed_path
return typeof p === 'string' && p ? p : undefined
} catch {
return undefined
}
}
function itemPath(args: any): string | undefined {
// write_trigger carries the item under `config` (see writeTriggerDraft), unlike
// every other tool here, which names it at the top level.
+19 -2
View File
@@ -1,4 +1,5 @@
import { get } from 'svelte/store'
import { SvelteMap } from 'svelte/reactivity'
import { onDestroy, untrack } from 'svelte'
import { deepEqual } from 'fast-equals'
import { workspaceStore } from './stores'
@@ -149,8 +150,24 @@ function noteMarker(set: Set<string>, key: string): void {
* whole resource — or a variable's decrypted secret — for the tab's lifetime.
*/
const releasedValues = new Map<string, unknown>()
/** Keys with a write in flight, by how many are. */
const settleWindows = new Map<string, number>()
/** Keys with a write in flight, by how many are. Reactive: it is also what the
* editors disable their Save on. */
const settleWindows = new SvelteMap<string, number>()
/**
* Whether a write is in flight for this cell — in any editor holding it, not just
* the one asking. Duplicate tabs and warm sessions mount several editors over one
* (workspace, kind, path), so a flag kept per editor serializes nothing: two can
* be writing the same item at once, and the older request landing last wins.
*/
export function isDraftSaving(
itemKind: UserDraftItemKind,
path: string | undefined,
opts?: UserDraftOptions
): boolean {
if (!path) return false
return settleWindows.has(mapKey(resolveWorkspace(opts), itemKind, path))
}
/**
* Open the window in which a release of this cell is worth remembering: the editor
@@ -18,7 +18,12 @@ vi.mock('./userDraftDbSyncer.svelte', () => ({
}
}))
import { settleDraftAfterWrite, UserDraft } from './userDraft.svelte'
import {
beginDraftSettleWindow,
isDraftSaving,
settleDraftAfterWrite,
UserDraft
} from './userDraft.svelte'
const stopSync = vi.fn()
const restartSync = vi.fn()
@@ -42,6 +47,29 @@ const sent = { path: 'u/me/a', value: 'sent' }
* newer edit by the time the write returns. Resetting it unconditionally — what
* every editor did — swallows that edit with no trace.
*/
// Duplicate tabs and warm sessions mount several editors over one cell, so what
// says "a write is in flight for this item" cannot live in any of them. The
// window each save opens is per key, and every editor's Save reads it.
describe('isDraftSaving', () => {
it('answers for the cell rather than the editor, until the last save closes', () => {
expect(isDraftSaving('variable', 'u/me/a', OPTS)).toBe(false)
const first = beginDraftSettleWindow('variable', 'u/me/a', OPTS)
const second = beginDraftSettleWindow('variable', 'u/me/a', OPTS)
expect(isDraftSaving('variable', 'u/me/a', OPTS)).toBe(true)
expect(isDraftSaving('variable', 'u/me/b', OPTS)).toBe(false)
first()
expect(isDraftSaving('variable', 'u/me/a', OPTS)).toBe(true)
second()
expect(isDraftSaving('variable', 'u/me/a', OPTS)).toBe(false)
})
// A create's form cell is routed nowhere, so nothing can collide with it.
it('is false for a path a cell cannot be keyed on', () => {
expect(isDraftSaving('variable', '', OPTS)).toBe(false)
expect(isDraftSaving('variable', undefined, OPTS)).toBe(false)
})
})
describe('settleDraftAfterWrite', () => {
it('resets the cell when it still holds what was written', () => {
settleDraftAfterWrite('variable', sent, { ...sent }, 'u/me/a', 'u/me/a', OPTS)
@@ -696,8 +696,8 @@
$effect(() => {
// Debounced so a burst of writes (the AI editing several files) reloads once.
setToolCompletionListener((name, args, workspace) => {
const { pages, entity, path } = toolReloadEffect(name, args)
setToolCompletionListener((name, args, workspace, result) => {
const { pages, entity, path, to } = toolReloadEffect(name, args, result)
if (pages.length === 0) return
for (const p of pages) pendingPages.add(pageReloadKey(workspace, p))
// A write reaches a hosted editor through the draft cell it holds, but an
@@ -719,7 +719,12 @@
name !== 'discard_local_draft' ||
!UserDraft.takeDraftOnlyDiscard(kind!, path!, { workspace })
const effect = effectForDiscard(effectForWrite(entity, seedReachedEditor), itemSurvives)
if (effect !== 'none') pendingMutations.push({ pages, effect, path, workspace })
// A mutation that moved the item is a rename like any other, so it goes
// through the same report an editor's own save makes: the tabs on the old
// path follow it, rather than being refreshed onto one that is now empty.
if (kind && path && to && to !== path) {
entityWritten({ kind, path, to, workspace, fromSessionId: undefined, fromTabId: '' })
} else if (effect !== 'none') pendingMutations.push({ pages, effect, path, workspace })
clearTimeout(reloadHandle)
reloadHandle = setTimeout(flushReload, 500)
})