mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 00:03:00 +00:00
fix: give a baseline-less payload a baseline before letting it go out
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
d1f57d5bd3
commit
860b63b2ea
@@ -95,6 +95,8 @@ export type ItemRowPort = {
|
||||
rowMark(key: ItemKey): number
|
||||
/** A row waiting for another attempt, if any: `undefined` when none, `null` for a delete. */
|
||||
pending(key: ItemKey): unknown | undefined
|
||||
/** Whether a row is waiting with no baseline behind it, so sending it would be unconditional. */
|
||||
unbased(key: ItemKey): boolean
|
||||
seedSync(key: ItemKey, draftSavedAt: string | undefined, since: number): void
|
||||
conflicted(key: ItemKey): boolean
|
||||
failure(key: ItemKey): string | undefined
|
||||
@@ -407,7 +409,10 @@ class Entry<V> {
|
||||
if (!adapter.load) throw new Error('This item cannot be loaded')
|
||||
// A row queued by whoever held this key before is debounced: send it, or this read
|
||||
// answers with the deployed value and the draft reappears under a clean editor.
|
||||
await this.ports.flush(key)
|
||||
// Not one written when no row existed, though — it carries no baseline, so it
|
||||
// would go out unconditional and take over a row created since. The read below
|
||||
// gives it one, and `pending` keeps showing it meanwhile.
|
||||
if (!this.ports.unbased(key)) await this.ports.flush(key)
|
||||
// Taken here, not when the command was asked for: rows this read's own flush sent
|
||||
// are in the response it is about to get. Only one handed over from now on, while
|
||||
// the read is out, leaves that response behind.
|
||||
@@ -1272,6 +1277,9 @@ const syncerRows: ItemRowPort = {
|
||||
pending(key) {
|
||||
return UserDraftDbSyncer.pendingValue(keyQuery(key))
|
||||
},
|
||||
unbased(key) {
|
||||
return UserDraftDbSyncer.hasUnbasedPending(keyQuery(key))
|
||||
},
|
||||
seedSync(key, draftSavedAt, since) {
|
||||
UserDraftDbSyncer.recordRemoteSync(keyQuery(key), draftSavedAt, since)
|
||||
},
|
||||
|
||||
@@ -37,6 +37,8 @@ function fakeRows() {
|
||||
const dropped: string[] = []
|
||||
/** Every `last_sync` baseline the syncer accepted, in order. */
|
||||
const seeds: { path: string; at: string | undefined }[] = []
|
||||
/** Paths the syncer holds a `last_sync` for. Without one a send is unconditional. */
|
||||
const baselines = new Set<string>()
|
||||
/** Rows handed over per path, by any writer — the store's reconcile or the chat's own save. */
|
||||
const sends = new Map<string, number>()
|
||||
const handed = (path: string) => sends.set(path, (sends.get(path) ?? 0) + 1)
|
||||
@@ -89,10 +91,13 @@ function fakeRows() {
|
||||
},
|
||||
rowMark: (key) => sends.get(key.path) ?? 0,
|
||||
pending: (key) => (parked.has(key.path) ? parked.get(key.path) : undefined),
|
||||
unbased: (key) => parked.has(key.path) && !baselines.has(key.path),
|
||||
seedSync: (key, at, since) => {
|
||||
// The syncer refuses a baseline from a read a row handed over since has passed.
|
||||
if (since !== (sends.get(key.path) ?? 0)) return
|
||||
seeds.push({ path: key.path, at })
|
||||
if (at !== undefined) baselines.add(key.path)
|
||||
else baselines.delete(key.path)
|
||||
// Back in sync with the server, as `recordRemoteSync` does. The baseline is fresh, so
|
||||
// a payload the server was refusing would be accepted from here on.
|
||||
conflicts.delete(key.path)
|
||||
@@ -1744,6 +1749,42 @@ describe('item store: conflicts', () => {
|
||||
expect(open.deployed).toEqual({ ...b, args: { a: 2 } })
|
||||
})
|
||||
|
||||
it('does not replay a first failed autosave over a draft created since', async () => {
|
||||
const rows = fakeRows()
|
||||
const store = createItemStore(rows.port)
|
||||
const key: ItemKey = { workspace: 'w', kind: 'resource', path: 'u/me/r' }
|
||||
// No draft exists, so nothing gives this key a baseline.
|
||||
const a = adapter(async () => ({
|
||||
deployed: deployedRes,
|
||||
draft: rows.sent.get('u/me/r') as Res | undefined
|
||||
}))
|
||||
const first = store.acquire(key, { workspace: 'w', path: 'u/me/r' }, a)
|
||||
await settle()
|
||||
rows.failing.add('u/me/r')
|
||||
first.handle.value = { ...deployedRes, description: 'tab A, never sent' }
|
||||
await rows.port.flush(key)
|
||||
first.release()
|
||||
|
||||
// Another tab creates the draft while tab A is away.
|
||||
const theirs = { ...deployedRes, description: 'tab B' }
|
||||
rows.sent.set('u/me/r', theirs)
|
||||
rows.handExternally('u/me/r')
|
||||
// Tab A's network recovers before it reopens.
|
||||
rows.failing.delete('u/me/r')
|
||||
|
||||
const second = store.acquire(key, { workspace: 'w', path: 'u/me/r' }, a)
|
||||
await settle()
|
||||
|
||||
// The parked payload has no `last_sync`, so replaying it before the read would go out
|
||||
// unconditional and take their draft with it — and the read after would see only the
|
||||
// overwrite, never a conflict.
|
||||
expect(rows.sent.get('u/me/r')).toEqual(theirs)
|
||||
// Tab A's edit is not lost either: it is what the reopened editor shows, and the read has
|
||||
// given the key a baseline, so when that payload does go out it is checked against theirs.
|
||||
expect(second.handle.value?.description).toBe('tab A, never sent')
|
||||
expect(rows.seeds.at(-1)?.path).toBe('u/me/r')
|
||||
})
|
||||
|
||||
it('keeps an edit the server never received when the item is reopened', async () => {
|
||||
const rows = fakeRows()
|
||||
const store = createItemStore(rows.port)
|
||||
|
||||
@@ -543,6 +543,16 @@ export const UserDraftDbSyncer = {
|
||||
return parked ? parked.value : undefined
|
||||
},
|
||||
|
||||
/**
|
||||
* Whether a payload is parked for `query` with no `last_sync` behind it — it was written when
|
||||
* no row existed, so sending it now would be unconditional and would take over a row created
|
||||
* since. A reader must get a baseline before letting one of these go out.
|
||||
*/
|
||||
hasUnbasedPending(query: UserDraftLastSyncQuery): boolean {
|
||||
const key = draftKey(query.workspace, query.itemKind, query.path)
|
||||
return pendingSaveOpts.has(key) && !lastSyncMap.has(key)
|
||||
},
|
||||
|
||||
/** Rows handed over for `query` so far, for a reader to hand back to `recordRemoteSync`. */
|
||||
sendsSoFar(query: UserDraftLastSyncQuery): number {
|
||||
return sends.get(draftKey(query.workspace, query.itemKind, query.path)) ?? 0
|
||||
|
||||
Reference in New Issue
Block a user