mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-29 16:15:51 +00:00
fix: give a deploy, an item deletion and a draft discard their own transitions
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
184af46829
commit
f28a6845e0
@@ -2325,6 +2325,7 @@ describe('global AI tools', () => {
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
// Claims the key and removes nothing, so anything still gone was deleted by the chat.
|
||||
discard: () => {
|
||||
discards++
|
||||
@@ -2341,6 +2342,7 @@ describe('global AI tools', () => {
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
@@ -2359,6 +2361,10 @@ describe('global AI tools', () => {
|
||||
read: () => undefined,
|
||||
refresh: () => {
|
||||
calls.push('refresh')
|
||||
return Promise.resolve(true)
|
||||
},
|
||||
itemDeleted: () => {
|
||||
calls.push('itemDeleted')
|
||||
return Promise.resolve()
|
||||
},
|
||||
discard: () => {
|
||||
@@ -2380,6 +2386,75 @@ describe('global AI tools', () => {
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
// A cleanup that the editor could not do — its first read still in flight, or failed — means
|
||||
// the row is nobody's, so the chat must still delete it rather than report success.
|
||||
it('deletes the draft itself when the live editor could not refresh', async () => {
|
||||
const path = 'u/admin/notyetloaded'
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
// Registered, but not loaded: it has not dealt with the row.
|
||||
refresh: () => Promise.resolve(false),
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
try {
|
||||
await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, {
|
||||
preserveLiveDraft: true,
|
||||
deployed: true
|
||||
})
|
||||
expect(getBackendDraft('resource', path)).toBeUndefined()
|
||||
} finally {
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => undefined,
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
// Deleting the deployed item leaves the editor with no baseline to reset to, so it is told
|
||||
// the item is gone rather than handed a draft discard.
|
||||
it('tells a live editor its item was deleted rather than discarding its draft', async () => {
|
||||
const path = 'u/admin/goneitem'
|
||||
seedBackendDraft('resource', path, { path, value: { a: 2 } })
|
||||
const calls: string[] = []
|
||||
registerLiveItemBridge({
|
||||
seed: () => false,
|
||||
read: () => undefined,
|
||||
refresh: () => Promise.resolve(true),
|
||||
itemDeleted: () => {
|
||||
calls.push('itemDeleted')
|
||||
return Promise.resolve()
|
||||
},
|
||||
discard: () => {
|
||||
calls.push('discard')
|
||||
return Promise.resolve({ removed: true })
|
||||
},
|
||||
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,
|
||||
itemDeleted: () => Promise.resolve(),
|
||||
discard: () => undefined,
|
||||
list: () => []
|
||||
})
|
||||
|
||||
@@ -8178,7 +8178,7 @@ async function deleteWorkspaceItem(
|
||||
// are no longer trustworthy (same rule as deploy success). Before the
|
||||
// draft cleanup: a cleanup failure must not leave stale comparisons.
|
||||
invalidateWorkspaceComparison(workspace)
|
||||
await deleteGlobalDraft(workspace, type, path, triggerKind)
|
||||
await deleteGlobalDraft(workspace, type, path, triggerKind, { itemDeleted: true })
|
||||
|
||||
// Record the deletion in the chat's modified-items mask. In a fork this leaves a
|
||||
// reviewable "removed" diff vs the parent that stays scoped to this chat. Keyed
|
||||
|
||||
@@ -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 {
|
||||
markLiveItemDeleted,
|
||||
refreshLiveItem,
|
||||
UserDraft,
|
||||
type UserDraftEntry,
|
||||
@@ -583,6 +584,9 @@ type DeleteGlobalDraftOptions = {
|
||||
* baseline from before that, and discarding the draft resets it to exactly that — showing
|
||||
* the pre-deploy value, clean, ready to be saved back over the deployment. */
|
||||
deployed?: boolean
|
||||
/** The deployed item itself was deleted. An editor open on it has nothing to reset to, so it
|
||||
* is told the item is gone rather than handed draft-discard semantics. */
|
||||
itemDeleted?: boolean
|
||||
}
|
||||
|
||||
export async function deleteGlobalDraft(
|
||||
@@ -595,19 +599,36 @@ export async function deleteGlobalDraft(
|
||||
const itemKind = itemKindFor(type, triggerKind)
|
||||
if (!itemKind) return
|
||||
const storagePath = resolveDraftStoragePath(workspace, itemKind, path)
|
||||
if (options.itemDeleted) {
|
||||
// Nothing to discard against: the item is gone. The editor is told that, and the row is
|
||||
// deleted here rather than by an editor that no longer has a baseline to discard to.
|
||||
await markLiveItemDeleted(workspace, itemKind, storagePath)
|
||||
UserDraft.forgetLocal(itemKind, storagePath, { workspace })
|
||||
await UserDraftDbSyncer.save({
|
||||
workspace,
|
||||
itemKind,
|
||||
path: storagePath,
|
||||
value: null,
|
||||
immediate: true
|
||||
})
|
||||
await assertDraftCleanupLanded(workspace, itemKind, path, storagePath)
|
||||
return
|
||||
}
|
||||
if (options.deployed) {
|
||||
const refreshing = refreshLiveItem(workspace, itemKind, storagePath)
|
||||
if (refreshing) {
|
||||
// Re-reading the open editor is the whole cleanup, and discarding on top of it is
|
||||
// wrong: the deploy carried the draft as it stood when it started, so anything typed
|
||||
// since is a divergence from what was deployed and belongs in the row. The editor's
|
||||
// own rule already drops the row when the two match and keeps it when they do not.
|
||||
await refreshing
|
||||
// Re-reading the open editor is the whole cleanup, and discarding on top of it is wrong:
|
||||
// the deploy carried the draft as it stood when it started, so anything typed since is a
|
||||
// divergence from what was deployed and belongs in the row. The editor's own rule drops
|
||||
// the row when the two match and keeps it when they do not.
|
||||
const refreshed = await refreshLiveItem(workspace, itemKind, storagePath)
|
||||
if (refreshed) {
|
||||
// Its reconcile only queued that; send it, so a caller awaiting this sees the server
|
||||
// settled — with no draft, or with the newer edit.
|
||||
await UserDraftDbSyncer.flush({ workspace, itemKind, path: storagePath })
|
||||
await assertDraftCleanupLanded(workspace, itemKind, path, storagePath)
|
||||
return
|
||||
}
|
||||
// It could not read after all (still loading, or the read failed), so it has not dealt
|
||||
// with the row and this caller still has to.
|
||||
}
|
||||
const liveDraft = UserDraft.getLiveEditorDraft(itemKind, { workspace })
|
||||
const live =
|
||||
@@ -630,9 +651,20 @@ export async function deleteGlobalDraft(
|
||||
immediate: true
|
||||
})
|
||||
}
|
||||
// A failed (network/5xx) or conflicted delete is recorded in the syncer state,
|
||||
// not thrown — surface it so callers don't report the draft as removed while
|
||||
// the DB-backed source of truth still has it (same guard as the write path).
|
||||
await assertDraftCleanupLanded(workspace, itemKind, path, storagePath)
|
||||
}
|
||||
|
||||
/**
|
||||
* A failed (network/5xx) or conflicted delete is recorded in the syncer state, not thrown —
|
||||
* surface it so callers don't report the draft as removed while the DB-backed source of truth
|
||||
* still has it (same guard as the write path), and only then invalidate the draft readers.
|
||||
*/
|
||||
async function assertDraftCleanupLanded(
|
||||
workspace: string,
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string,
|
||||
storagePath: string
|
||||
): Promise<void> {
|
||||
const state = UserDraftDbSyncer.getState({ workspace, itemKind, path: storagePath })
|
||||
if (state.state === 'failed') {
|
||||
throw new Error(state.failureMessage ?? `Failed to delete draft "${path}".`)
|
||||
|
||||
@@ -446,6 +446,35 @@ class Entry<V> {
|
||||
return this.load(this.adapter, asOf, true)
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-read for a caller that has just written the deployed item, taking a turn in the queue
|
||||
* first: asked for during the first load, `reread` would find nothing loaded and return
|
||||
* without doing anything. Resolves false when it could not read after all, so the caller
|
||||
* knows this editor has not dealt with the row and it must.
|
||||
*/
|
||||
async refreshed(): Promise<boolean> {
|
||||
await this.run(async () => {})
|
||||
if (!this.loaded || this.retired || !this.adapter) return false
|
||||
return (await this.reread()).ok
|
||||
}
|
||||
|
||||
/**
|
||||
* The deployed item itself was deleted elsewhere. Not a discard: there is no baseline left to
|
||||
* reset to, so the editor reports it gone rather than sitting clean on something whose next
|
||||
* save would 404. The server removes the row with the item, so none is written.
|
||||
*/
|
||||
itemDeleted(): Promise<void> {
|
||||
return this.run(async () => {
|
||||
if (this.retired || !this.loaded) return
|
||||
this.serverDeployed = undefined
|
||||
this.patched = {}
|
||||
this.replaceValue(undefined)
|
||||
this.row = null
|
||||
this.removed = true
|
||||
this.ports.hint(this.key, false)
|
||||
})
|
||||
}
|
||||
|
||||
/** An outside write (the AI chat, another editor): a real divergence, never settling. */
|
||||
applyExternal(value: V): number {
|
||||
if (this.loaded && serialize(value) === serialize(this.value)) return this.revision
|
||||
@@ -1093,8 +1122,11 @@ export function createItemStore(ports: ItemRowPort) {
|
||||
workspace: string,
|
||||
kind: UserDraftItemKind,
|
||||
path: string
|
||||
): Promise<unknown> | undefined {
|
||||
return find(workspace, kind, path)?.reread()
|
||||
): Promise<boolean> | undefined {
|
||||
return find(workspace, kind, path)?.refreshed()
|
||||
},
|
||||
itemDeleted(workspace: string, kind: UserDraftItemKind, path: string): Promise<void> {
|
||||
return find(workspace, kind, path)?.itemDeleted() ?? Promise.resolve()
|
||||
},
|
||||
discard(
|
||||
workspace: string,
|
||||
|
||||
@@ -752,6 +752,28 @@ describe('item store: origins', () => {
|
||||
expect(item.value).toEqual(typed)
|
||||
})
|
||||
|
||||
it('reports an item deleted elsewhere as gone, not as clean', async () => {
|
||||
const rows = fakeRows()
|
||||
const store = createItemStore(rows.port)
|
||||
const { handle: item } = store.acquire(
|
||||
{ workspace: 'w', kind: 'resource', path: 'u/me/r' },
|
||||
{ workspace: 'w', path: 'u/me/r' },
|
||||
adapter({ deployed: deployedRes })
|
||||
)
|
||||
await settle()
|
||||
item.value = { ...deployedRes, description: 'edited' }
|
||||
expect(item.dirty).toBe(true)
|
||||
|
||||
await store.bridge.itemDeleted('w', 'resource', 'u/me/r')
|
||||
|
||||
// Resetting to the deployed baseline would leave the editor clean on something that no
|
||||
// longer exists, and its next save would 404.
|
||||
expect(item.removed).toBe(true)
|
||||
expect(item.value).toBeUndefined()
|
||||
expect(item.deployed).toBeUndefined()
|
||||
expect(rows.hints.get('u/me/r')).toBe(false)
|
||||
})
|
||||
|
||||
it('keeps a draft-only item an outside write put back while its delete was going', async () => {
|
||||
const rows = fakeRows()
|
||||
const draft = { ...deployedRes, description: 'only a draft' }
|
||||
|
||||
@@ -147,7 +147,9 @@ export type LiveItemBridge = {
|
||||
workspace: string,
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string
|
||||
): Promise<unknown> | undefined
|
||||
): Promise<boolean> | undefined
|
||||
/** The deployed item was deleted: the live item reports itself gone. */
|
||||
itemDeleted(workspace: string, itemKind: UserDraftItemKind, path: string): Promise<void>
|
||||
/** Discard the live item's draft, resolving once its delete has landed. `undefined` when no
|
||||
* live item holds the key, so the caller issues the delete itself. */
|
||||
discard(
|
||||
@@ -166,10 +168,20 @@ export function refreshLiveItem(
|
||||
workspace: string,
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string
|
||||
): Promise<unknown> | undefined {
|
||||
): Promise<boolean> | undefined {
|
||||
return liveItems?.refresh(workspace, itemKind, path)
|
||||
}
|
||||
|
||||
/** Tell a live item that the deployed item under it has been deleted, so it stops showing a
|
||||
* baseline that no longer exists. Resolves once it has. */
|
||||
export function markLiveItemDeleted(
|
||||
workspace: string,
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string
|
||||
): Promise<void> {
|
||||
return liveItems?.itemDeleted(workspace, itemKind, path) ?? Promise.resolve()
|
||||
}
|
||||
|
||||
export function registerLiveItemBridge(bridge: LiveItemBridge): void {
|
||||
liveItems = bridge
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user