diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index e61088812c..eb83af7ba2 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -180,8 +180,10 @@ class Entry { /** 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 | undefined /** The entries a save of this entry waits for before moving: a move skips waiting for any @@ -346,6 +348,8 @@ class Entry { /** 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 { carryEditsOf(old: Entry): 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 { 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 }, diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index 13e7a79fef..a8b51ad972 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -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, 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()