starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-10-14 00:02:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: tell a failed refresh from an absent editor, and a patch's value from a newer one

Browse Source 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-15 16:18:53 +02:00
co-authored by Claude Opus 5
parent 8a948f127b
commit da588bf8ed
5 changed files with 86 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/lib/components/copilot/chat/global/core.test.ts
+4 -4
View File
@@ -2361,7 +2361,7 @@ describe('global AI tools', () => {
read: () => undefined,
refresh: () => {
calls.push('refresh')
return Promise.resolve(true)
return Promise.resolve('done' as const)
},
itemDeleted: () => {
calls.push('itemDeleted')
@@ -2401,8 +2401,8 @@ describe('global AI tools', () => {
registerLiveItemBridge({
seed: () => false,
read: () => undefined,
// Registered, but not loaded: it answers, and says it has not dealt with the row.
refresh: () => Promise.resolve(false),
// Registered, but never had the item: it has not dealt with the row.
refresh: () => Promise.resolve('absent' as const),
itemDeleted: () => Promise.resolve(),
discard: () => Promise.resolve(false),
list: () => []
@@ -2434,7 +2434,7 @@ describe('global AI tools', () => {
registerLiveItemBridge({
seed: () => false,
read: () => undefined,
refresh: () => Promise.resolve(true),
refresh: () => Promise.resolve('done' as const),
itemDeleted: () => {
calls.push('itemDeleted')
return Promise.resolve()
frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts
+10 -3
View File
@@ -620,15 +620,22 @@ export async function deleteGlobalDraft(
// 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) {
if (refreshed === 'done') {
// 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.
if (refreshed === 'failed') {
// It has the item and a baseline from before the deploy. Discarding its row would
// reset it to that and throw away anything typed during the deploy, so the deploy is
// reported as done and its draft cleanup as not, which is what happened.
throw new Error(
`Deployed "${path}", but its open editor could not be refreshed, so its draft was left in place. Reload the editor.`
)
}
// `absent`: never had the item, so it has not dealt with the row and this caller must.
}
const liveDraft = UserDraft.getLiveEditorDraft(itemKind, { workspace })
const live =
frontend/src/lib/itemStore.svelte.ts
+17 -8
View File
@@ -451,13 +451,16 @@ class Entry<V> {
/**
* 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.
* without doing anything. The three answers are not interchangeable — `absent` means this
* editor never had the item and the row is the caller's to deal with, while `failed` means it
* has the item and an out-of-date baseline, so anything resetting to that baseline now would
* undo what was just deployed.
*/
async refreshed(): Promise<boolean> {
async refreshed(): Promise<'done' | 'absent' | 'failed'> {
await this.run(async () => {})
if (!this.loaded || this.retired || !this.adapter) return false
return (await this.reread()).ok
if (this.retired || !this.adapter) return 'absent'
if (!this.loaded) return 'absent'
return (await this.reread()).ok ? 'done' : 'failed'
}
/**
@@ -683,6 +686,10 @@ class Entry<V> {
}
if (this.value !== undefined) Object.assign(this.value as object, fields)
this.touch()
// Taken after this patch's own touch, so it counts only what happens next. The value it
// put on screen is the user's copy, and a field carrying the value this patch sent is not
// proof this patch put it there: the chat can write a draft holding the same value.
const appliedAt = this.edits
/** Release each field this patch still owns; one a later patch has taken stays with it. */
const release = () => {
for (const k of Object.keys(sent)) {
@@ -704,12 +711,14 @@ class Entry<V> {
}
release()
// The value is the user's copy, so it is given back by hand — to the baseline as it
// stands now, which is the server's value or a later patch's, never this one's.
// stands now, which is the server's value or a later patch's, never this one's. Not
// given back at all if something has written the value since: that is newer than this
// patch and rolling it back would be this patch overwriting it.
if (failed) {
const base = (onTemplate ? this.template : this.deployed) as
| Record<string, unknown>
| undefined
if (base !== undefined && this.value !== undefined) {
if (base !== undefined && this.value !== undefined && this.edits === appliedAt) {
const out = snapshot(this.value) as Record<string, unknown>
let changed = false
for (const [k, v] of Object.entries(sent)) {
@@ -1137,7 +1146,7 @@ export function createItemStore(ports: ItemRowPort) {
workspace: string,
kind: UserDraftItemKind,
path: string
): Promise<boolean> | undefined {
): Promise<'done' | 'absent' | 'failed'> | undefined {
return find(workspace, kind, path)?.refreshed()
},
itemDeleted(workspace: string, kind: UserDraftItemKind, path: string): Promise<void> {
frontend/src/lib/itemStore.test.ts
+51 -1
View File
@@ -785,6 +785,56 @@ describe('item store: origins', () => {
})
})
it('tells a caller its post-deploy re-read failed, apart from never having the item', async () => {
const rows = fakeRows()
const store = createItemStore(rows.port)
let reads = 0
const { handle: item } = store.acquire(
{ workspace: 'w', kind: 'resource', path: 'u/me/r' },
{ workspace: 'w', path: 'u/me/r' },
adapter(async () => {
// The first read works, so this editor has the item; the one after the deploy does not.
if (reads++ > 0) throw new Error('the GET failed')
return { deployed: deployedRes }
})
)
await settle()
item.value = { ...deployedRes, description: 'typed during the deploy' }
// `absent` would send the caller off to delete this row and discard against a baseline
// from before the deploy; the editor still has the item, so it says so.
expect(await store.bridge.refresh('w', 'resource', 'u/me/r')).toBe('failed')
expect(item.value?.description).toBe('typed during the deploy')
expect(item.dirty).toBe(true)
})
it('does not roll a refused toggle back over a draft written while it was out', async () => {
type Sched = { path: string; enabled: boolean; summary: string }
const rows = fakeRows()
const store = createItemStore(rows.port)
const refusal = deferred()
const { handle: item } = store.acquire(
{ workspace: 'w', kind: 'trigger_schedule', path: 's' },
{ workspace: 'w', path: 's' },
{
load: async () => ({ deployed: { path: 's', enabled: false, summary: 'a' } })
} as ItemAdapter<Sched>
)
await settle()
const toggling = item.patch({ enabled: true }, () => refusal.promise)
// The chat writes a draft that happens to carry the same `enabled` the toggle sent.
item.applyExternal({ path: 's', enabled: true, summary: 'from the chat' })
refusal.reject(new Error('refused'))
expect(await toggling).toMatchObject({ ok: false })
// Equality with what the toggle sent is not proof the toggle put it there.
expect(item.value).toEqual({ path: 's', enabled: true, summary: 'from the chat' })
expect(rows.writes.at(-1)).toEqual({
path: 's',
value: { path: 's', enabled: true, summary: 'from the chat' }
})
})
it('says it did not deal with the row when its own first read never arrived', async () => {
const rows = fakeRows()
const store = createItemStore(rows.port)
@@ -801,7 +851,7 @@ describe('item store: origins', () => {
// It is registered, so the chat would otherwise take its answer as the cleanup being done
// — while an entry with nothing loaded writes no delete at all and the row survives.
expect(await store.bridge.discard('w', 'resource', 'u/me/r')).toBe(false)
expect(await store.bridge.refresh('w', 'resource', 'u/me/r')).toBe(false)
expect(await store.bridge.refresh('w', 'resource', 'u/me/r')).toBe('absent')
expect(rows.writes).toEqual([])
})
frontend/src/lib/userDraft.svelte.ts
+4 -3
View File
@@ -142,12 +142,13 @@ export type LiveItemBridge = {
path: string
): { value: unknown | undefined } | undefined
/** Re-read the live item, for a caller that has just written the deployed item under it.
* `undefined` when no live item holds the key. */
* `undefined` when no live item holds the key; `absent` when one is registered but never had
* the item; `failed` when it has the item but could not re-read, so its baseline is stale. */
refresh(
workspace: string,
itemKind: UserDraftItemKind,
path: string
): Promise<boolean> | undefined
): Promise<'done' | 'absent' | 'failed'> | 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 to whether it dealt with the row. `undefined` when
@@ -169,7 +170,7 @@ export function refreshLiveItem(
workspace: string,
itemKind: UserDraftItemKind,
path: string
): Promise<boolean> | undefined {
): Promise<'done' | 'absent' | 'failed'> | undefined {
return liveItems?.refresh(workspace, itemKind, path)
}