mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: let the last outside write at a key win over a discard and a parked draft
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015vn1jCHUcAdaG9C4NsUehF
This commit is contained in:
co-authored by
Claude Opus 5
parent
4ffa65538f
commit
2d48f7f0f9
@@ -180,8 +180,10 @@ class Entry<V> {
|
||||
/** Replaced by an entry that moved onto its key: it no longer owns the row there, and a write
|
||||
* reaching its turn does nothing, as its handles show the item that replaced it. */
|
||||
retired = false
|
||||
/** Discards asked and still waiting for their turn. */
|
||||
/** Discards asked and still waiting for their turn, and whether an outside write has landed
|
||||
* since, which outranks them. */
|
||||
discardsAsked = 0
|
||||
private writtenSinceDiscard = false
|
||||
/** The command running now, past its turn: what a move onto this key waits for. */
|
||||
running: Promise<unknown> | undefined
|
||||
/** The entries a save of this entry waits for before moving: a move skips waiting for any
|
||||
@@ -346,6 +348,8 @@ class Entry<V> {
|
||||
/** 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
|
||||
// A draft written again after a discard was asked outranks it: the last outside write wins.
|
||||
if (this.discardsAsked > 0) this.writtenSinceDiscard = true
|
||||
this.pristine = false
|
||||
this.removed = false
|
||||
this.replaceValue(value)
|
||||
@@ -425,7 +429,8 @@ class Entry<V> {
|
||||
carryEditsOf(old: Entry<V>): void {
|
||||
// A discard asked before the move landed wins over the edits it was asked to drop; had it
|
||||
// run first, it would have dropped anything typed after it too.
|
||||
if (old.value === undefined || this.value === undefined || old.discardsAsked > 0) return
|
||||
if (old.value === undefined || this.value === undefined) return
|
||||
if (old.discardsAsked > 0 && !old.writtenSinceDiscard) return
|
||||
// Not loaded yet (its read waits behind the move): only an outside write can have filled it,
|
||||
// and nothing has persisted that write but this entry.
|
||||
if (!old.loaded) {
|
||||
@@ -467,6 +472,7 @@ class Entry<V> {
|
||||
this.discardsAsked++
|
||||
return this.run(async () => {
|
||||
this.discardsAsked--
|
||||
if (this.discardsAsked === 0) this.writtenSinceDiscard = false
|
||||
if (!this.loaded || this.retired) return { removed: false }
|
||||
if (this.origin === 'draft') {
|
||||
const kept = snapshot(this.value)
|
||||
@@ -973,11 +979,13 @@ export function createItemStore(ports: ItemRowPort) {
|
||||
return { value: entry.dirty && entry.origin !== 'new' ? snapshot(entry.value) : undefined }
|
||||
},
|
||||
discard(workspace: string, kind: UserDraftItemKind, path: string): boolean {
|
||||
// The discard is asked for the key, whoever holds it: a draft parked for the save moving
|
||||
// onto it is discarded too, however it was parked.
|
||||
// The discard is asked for the key, whoever holds it: a draft parked for a save moving
|
||||
// onto that key goes with it, however it was parked.
|
||||
arrivals.delete(keyString({ workspace, kind: kind as ItemKind, path }))
|
||||
const entry = find(workspace, kind, path)
|
||||
if (!entry) {
|
||||
arrivals.delete(keyString({ workspace, kind: kind as ItemKind, path }))
|
||||
return false
|
||||
}
|
||||
if (!entry) return false
|
||||
void entry.discard()
|
||||
return true
|
||||
},
|
||||
|
||||
@@ -616,6 +616,58 @@ describe('item store: one entry per key', () => {
|
||||
expect(rows.writes.at(-1)).toEqual({ path: 'u/me/b', value: null })
|
||||
})
|
||||
|
||||
it('keeps a draft written again after a discard, and drops one parked for the move', async () => {
|
||||
const b = { ...deployedRes, path: 'u/me/b' }
|
||||
const moveOnto = async (
|
||||
openBefore: boolean,
|
||||
outside: (store: ReturnType<typeof createItemStore>, open: () => void) => void
|
||||
) => {
|
||||
const rows = fakeRows()
|
||||
const store = createItemStore(rows.port)
|
||||
const gate = deferred()
|
||||
const temporary = newItemPath()
|
||||
const { handle: moving } = store.acquire(
|
||||
{ workspace: 'w', kind: 'resource', path: temporary },
|
||||
{ workspace: 'w', path: temporary, template: b },
|
||||
adapter({}, () => gate.promise)
|
||||
)
|
||||
await settle()
|
||||
moving.value = { ...b, args: { a: 2 } }
|
||||
const moved = moving.save()
|
||||
await settle()
|
||||
const open = () =>
|
||||
store.acquire(
|
||||
{ workspace: 'w', kind: 'resource', path: 'u/me/b' },
|
||||
{ workspace: 'w', path: 'u/me/b' },
|
||||
adapter({ deployed: b })
|
||||
)
|
||||
if (openBefore) open()
|
||||
outside(store, open)
|
||||
gate.resolve()
|
||||
expect(await moved).toMatchObject({ ok: true, moved: true })
|
||||
return { moving, rows }
|
||||
}
|
||||
// Deleted, then written again: the newer draft is the one that survives the move.
|
||||
const again = { ...b, description: 'written again after the delete' }
|
||||
const first = await moveOnto(true, (store) => {
|
||||
store.bridge.seed('w', 'resource', 'u/me/b', { ...b, description: 'first draft' })
|
||||
store.bridge.discard('w', 'resource', 'u/me/b')
|
||||
store.bridge.seed('w', 'resource', 'u/me/b', again)
|
||||
})
|
||||
expect(first.moving.value).toEqual(again)
|
||||
expect(first.rows.writes.at(-1)).toEqual({ path: 'u/me/b', value: again })
|
||||
|
||||
// Parked for the move before anyone held the key, then deleted once an editor does: the
|
||||
// discard is asked for the key, so the parked draft goes with it.
|
||||
const second = await moveOnto(false, (store, open) => {
|
||||
store.bridge.seed('w', 'resource', 'u/me/b', { ...b, description: 'parked draft' })
|
||||
open()
|
||||
store.bridge.discard('w', 'resource', 'u/me/b')
|
||||
})
|
||||
expect(second.moving.dirty).toBe(false)
|
||||
expect(second.rows.writes.at(-1)).toEqual({ path: 'u/me/b', value: null })
|
||||
})
|
||||
|
||||
it('keeps a change the draft comparison ignores in an editor a move replaces', async () => {
|
||||
type Sched = { path: string; summary: string; permissioned_as?: string }
|
||||
const rows = fakeRows()
|
||||
|
||||
Reference in New Issue
Block a user