mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 16:02:59 +00:00
fix: persist the row a live item keeps, not the value handed to it
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
e58ba737e4
commit
ec157d628c
@@ -345,7 +345,8 @@ import {
|
||||
import {
|
||||
UserDraft,
|
||||
__resetUserDraftForTesting,
|
||||
registerLiveItemBridge
|
||||
registerLiveItemBridge,
|
||||
type LiveItemBridge
|
||||
} from '$lib/userDraft.svelte'
|
||||
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
|
||||
import {
|
||||
@@ -2310,6 +2311,39 @@ describe('global AI tools', () => {
|
||||
expect(res.status).toBe('conflict')
|
||||
})
|
||||
|
||||
/** A bridge with nobody holding any key. Spread it and override only what the test is about, so
|
||||
* a member added to the bridge later cannot quietly answer for a fixture that never meant to. */
|
||||
function noLiveItems(): LiveItemBridge {
|
||||
return {
|
||||
seed: () => false,
|
||||
rowFor: () => undefined,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
}
|
||||
}
|
||||
|
||||
// The chat can write a value that turns out to be the deployed one. A live editor takes it and
|
||||
// stays clean, keeping no row — so this save has to clear the row rather than persist a draft
|
||||
// its own editor does not have: nothing would show it, and nothing short of an edit would
|
||||
// remove it, while it still lists and goes stale once the deployed item moves on.
|
||||
it('clears the row when the chat writes the value the live editor already deploys', async () => {
|
||||
const path = 'f/res/noop'
|
||||
const deployed = { path, value: { a: 1 } }
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
registerLiveItemBridge({ ...noLiveItems(), seed: () => true, rowFor: () => null })
|
||||
try {
|
||||
const res = await persistGlobalDraft(WORKSPACE, 'resource', path, deployed)
|
||||
expect(res.status).toBe('saved')
|
||||
expect(getBackendDraft('resource', path)).toBeUndefined()
|
||||
} finally {
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
// An open editor owns its own row: the chat must defer to that editor's discard rather than
|
||||
// send a delete of its own, which would be a second unconditional request.
|
||||
it('leaves the delete to a live editor that owns the draft', async () => {
|
||||
@@ -2322,32 +2356,19 @@ describe('global AI tools', () => {
|
||||
})
|
||||
let discards = 0
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
...noLiveItems(),
|
||||
// Claims the key and removes nothing, so anything still gone was deleted by the chat.
|
||||
discard: () => {
|
||||
discards++
|
||||
return Promise.resolve('done' as const)
|
||||
},
|
||||
list: () => []
|
||||
}
|
||||
})
|
||||
try {
|
||||
await deleteGlobalDraft(WORKSPACE, 'script', path)
|
||||
expect(discards).toBe(1)
|
||||
expect(getBackendDraft('script', path)).toBeDefined()
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
@@ -2359,13 +2380,11 @@ describe('global AI tools', () => {
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
const calls: string[] = []
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
...noLiveItems(),
|
||||
refresh: () => {
|
||||
calls.push('refresh')
|
||||
return Promise.resolve('done' as const)
|
||||
},
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => {
|
||||
calls.push('itemDeleted')
|
||||
return Promise.resolve()
|
||||
@@ -2373,8 +2392,7 @@ describe('global AI tools', () => {
|
||||
discard: () => {
|
||||
calls.push('discard')
|
||||
return Promise.resolve({ removed: true })
|
||||
},
|
||||
list: () => []
|
||||
}
|
||||
})
|
||||
try {
|
||||
await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, {
|
||||
@@ -2385,15 +2403,7 @@ describe('global AI tools', () => {
|
||||
// The chat sends no delete of its own either: the row is the editor's to settle.
|
||||
expect(getBackendDraft('resource', path)).toBeDefined()
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
@@ -2403,14 +2413,10 @@ describe('global AI tools', () => {
|
||||
const path = 'u/admin/notyetloaded'
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
...noLiveItems(),
|
||||
// Registered, but never had the item: it has not dealt with the row.
|
||||
refresh: () => Promise.resolve('absent' as const),
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => Promise.resolve('absent' as const),
|
||||
list: () => []
|
||||
discard: () => Promise.resolve('absent' as const)
|
||||
})
|
||||
try {
|
||||
await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, {
|
||||
@@ -2419,15 +2425,7 @@ describe('global AI tools', () => {
|
||||
})
|
||||
expect(getBackendDraft('resource', path)).toBeUndefined()
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
@@ -2438,10 +2436,8 @@ describe('global AI tools', () => {
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
const calls: string[] = []
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
...noLiveItems(),
|
||||
refresh: () => Promise.resolve('done' as const),
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => {
|
||||
calls.push('itemDeleted')
|
||||
return Promise.resolve()
|
||||
@@ -2449,23 +2445,14 @@ describe('global AI tools', () => {
|
||||
discard: () => {
|
||||
calls.push('discard')
|
||||
return Promise.resolve('done' as const)
|
||||
},
|
||||
list: () => []
|
||||
}
|
||||
})
|
||||
try {
|
||||
await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, { itemDeleted: true })
|
||||
expect(calls).toEqual(['itemDeleted'])
|
||||
expect(getBackendDraft('resource', path)).toBeUndefined()
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
@@ -2476,27 +2463,15 @@ describe('global AI tools', () => {
|
||||
const path = 'u/admin/stale_res'
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
...noLiveItems(),
|
||||
refresh: () => Promise.resolve('failed' as const),
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => Promise.resolve('failed' as const),
|
||||
list: () => []
|
||||
discard: () => Promise.resolve('failed' as const)
|
||||
})
|
||||
try {
|
||||
await expect(deleteGlobalDraft(WORKSPACE, 'resource', path)).rejects.toThrow()
|
||||
expect(getBackendDraft('resource', path)).toBeDefined()
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
@@ -2507,13 +2482,11 @@ describe('global AI tools', () => {
|
||||
const path = 'u/admin/kept_res'
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
...noLiveItems(),
|
||||
// The editor kept a row: it diverged from what was deployed while the deploy ran.
|
||||
read: () => ({ value: { path, value: { a: 3 } } }),
|
||||
refresh: () => Promise.resolve('done' as const),
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => Promise.resolve('done' as const),
|
||||
list: () => []
|
||||
discard: () => Promise.resolve('done' as const)
|
||||
})
|
||||
try {
|
||||
const cleanup = await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, {
|
||||
@@ -2522,15 +2495,7 @@ describe('global AI tools', () => {
|
||||
})
|
||||
expect(cleanup).toEqual({ removed: false })
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
@@ -2540,28 +2505,16 @@ describe('global AI tools', () => {
|
||||
const path = 'u/admin/kept_on_discard'
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
...noLiveItems(),
|
||||
// Still holding a draft after the discard: the user typed after it was asked for.
|
||||
read: () => ({ value: { path, value: { a: 3 } } }),
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => Promise.resolve('done' as const),
|
||||
list: () => []
|
||||
discard: () => Promise.resolve('done' as const)
|
||||
})
|
||||
try {
|
||||
const out = await deleteGlobalDraft(WORKSPACE, 'resource', path)
|
||||
expect(out).toEqual({ removed: false })
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
@@ -2573,11 +2526,8 @@ describe('global AI tools', () => {
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
let discards = 0
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
...noLiveItems(),
|
||||
read: () => ({ value: { path, value: { a: 3 } } }),
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => {
|
||||
discards++
|
||||
return Promise.resolve('done' as const)
|
||||
@@ -2592,15 +2542,7 @@ describe('global AI tools', () => {
|
||||
clearGlobalDrafts(WORKSPACE, new Set([path]))
|
||||
expect(discards).toBe(1)
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
noteRow: () => {},
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
registerLiveItemBridge(noLiveItems())
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ import { DraftService } from '$lib/gen'
|
||||
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
|
||||
import { DEFAULT_DATA as DEFAULT_RAW_APP_DATA } from '$lib/components/raw_apps/dataTableRefUtils'
|
||||
import {
|
||||
liveItemRow,
|
||||
markLiveItemDeleted,
|
||||
noteLiveItemRow,
|
||||
refreshLiveItem,
|
||||
@@ -428,7 +429,13 @@ export async function persistGlobalDraft(
|
||||
const itemKind = itemKindFor(type, opts.triggerKind)
|
||||
if (!itemKind) throw new Error(`Unsupported draft type "${type}".`)
|
||||
const storagePath = resolveDraftStoragePath(workspace, itemKind, path)
|
||||
UserDraft.seed(itemKind, storagePath, value, { workspace })
|
||||
// A live item that took the value decides what belongs in the row, and answers `null` when the
|
||||
// value it now holds is the deployed one. Persisting `value` regardless would leave a row for a
|
||||
// draft that item does not have, invisible to its banner and undeletable by its Save. Asked
|
||||
// only of an item that took this value, so the answer is about it and not what it held before.
|
||||
const took = UserDraft.seed(itemKind, storagePath, value, { workspace })
|
||||
const held = took ? liveItemRow(workspace, itemKind, storagePath) : undefined
|
||||
const row = held === undefined ? value : held === null ? null : held.value
|
||||
// An open editor took that value and may have written its own row for it; this save is a
|
||||
// second row for the same value, so say so rather than leave it looking like someone else's.
|
||||
noteLiveItemRow(workspace, itemKind, storagePath)
|
||||
@@ -436,7 +443,7 @@ export async function persistGlobalDraft(
|
||||
workspace,
|
||||
itemKind,
|
||||
path: storagePath,
|
||||
value,
|
||||
value: row,
|
||||
immediate: true,
|
||||
force: opts.force
|
||||
})
|
||||
@@ -768,10 +775,7 @@ export async function flushGlobalDraftSaves(
|
||||
* changed after that discard was asked for — would have that newer edit taken on this pass,
|
||||
* undoing exactly what the first one preserved. `keep` names the paths to leave alone.
|
||||
*/
|
||||
export function clearGlobalDrafts(
|
||||
workspace: string,
|
||||
keep: ReadonlySet<string> = new Set()
|
||||
): void {
|
||||
export function clearGlobalDrafts(workspace: string, keep: ReadonlySet<string> = new Set()): void {
|
||||
for (const draft of UserDraft.list({ workspace, itemKinds: [...GLOBAL_DRAFT_KINDS] })) {
|
||||
if (keep.has(draft.path)) continue
|
||||
UserDraft.forgetLocal(draft.itemKind, draft.path, { workspace })
|
||||
|
||||
@@ -1242,6 +1242,19 @@ export function createItemStore(ports: ItemRowPort) {
|
||||
entry.applyExternal(value)
|
||||
return true
|
||||
},
|
||||
/** The row this entry keeps for the key, by the same rule `reconcile` writes by: `{ value }`
|
||||
* when its value diverges from the deployed one, `null` when it does not and so nothing
|
||||
* belongs in the row. `undefined` when nobody can answer — no entry, one that has not read
|
||||
* yet, or one whose baseline is known to be behind the server. */
|
||||
rowFor(
|
||||
workspace: string,
|
||||
kind: UserDraftItemKind,
|
||||
path: string
|
||||
): { value: unknown } | null | undefined {
|
||||
const entry = find(workspace, kind, path)
|
||||
if (!entry || !entry.loaded || entry.stale) return undefined
|
||||
return entry.dirty ? { value: snapshot(entry.value) } : null
|
||||
},
|
||||
/** The draft as this tab knows it: the value when it diverges, nothing otherwise. */
|
||||
read(
|
||||
workspace: string,
|
||||
|
||||
@@ -135,6 +135,13 @@ const liveEditorDrafts = new Map<string, LiveEditorDraft>()
|
||||
export type LiveItemBridge = {
|
||||
/** Apply `value` as an outside write. False when no live item holds the key. */
|
||||
seed(workspace: string, itemKind: UserDraftItemKind, path: string, value: unknown): boolean
|
||||
/** The row a live item keeps for the key: `{ value }` when its value diverges from the deployed
|
||||
* one, `null` when it does not. `undefined` when no live item can answer. */
|
||||
rowFor(
|
||||
workspace: string,
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string
|
||||
): { value: unknown } | null | undefined
|
||||
/** `undefined` when no live item holds the key; else the draft, if it has one. */
|
||||
read(
|
||||
workspace: string,
|
||||
@@ -176,6 +183,18 @@ export function refreshLiveItem(
|
||||
return liveItems?.refresh(workspace, itemKind, path)
|
||||
}
|
||||
|
||||
/** What a live item keeps in the key's row, for a caller about to write that row itself: the
|
||||
* store's rule is that a row exists exactly while the value diverges from the deployed one, so
|
||||
* persisting the raw value instead would leave a row behind for a value that matches it.
|
||||
* `undefined` when no live item can answer and the caller's own value is all there is. */
|
||||
export function liveItemRow(
|
||||
workspace: string,
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string
|
||||
): { value: unknown } | null | undefined {
|
||||
return liveItems?.rowFor(workspace, itemKind, path)
|
||||
}
|
||||
|
||||
/** Tell a live item that a row just written for it was written by whoever handed it that same
|
||||
* value through `seed`, not by some other editor whose value it does not have. */
|
||||
export function noteLiveItemRow(
|
||||
@@ -537,11 +556,15 @@ export const UserDraft = {
|
||||
* editor's `initContent` cascading into the bound value).
|
||||
*
|
||||
* No-op if the entry isn't live yet (acquire via `use`/`useMany` first).
|
||||
*
|
||||
* Returns whether a live item took the value, for a caller about to persist it: only then does
|
||||
* `liveItemRow` describe that value rather than whatever the item held already.
|
||||
*/
|
||||
seed<V>(itemKind: UserDraftItemKind, path: string, value: V, opts?: UserDraftOptions): void {
|
||||
seed<V>(itemKind: UserDraftItemKind, path: string, value: V, opts?: UserDraftOptions): boolean {
|
||||
const ws = resolveWorkspace(opts)
|
||||
liveItems?.seed(ws, itemKind, path, value)
|
||||
const took = liveItems?.seed(ws, itemKind, path, value) ?? false
|
||||
seedLocalMirror(mapKey(ws, itemKind, path), value)
|
||||
return took
|
||||
},
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user