mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 00:03:00 +00:00
fix: let an unasked read move the deployed side under a conflict without answering it
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
a3e0e6e80d
commit
d491693a8a
@@ -353,7 +353,7 @@ class Entry<V> {
|
||||
|
||||
/** `editedBefore`: the edit count as of when this read was decided on, for a caller that did
|
||||
* something async first. */
|
||||
load(adapter: ItemAdapter<V>, asOf?: AsOf): Promise<CommandOutcome> {
|
||||
load(adapter: ItemAdapter<V>, asOf?: AsOf, unasked = false): Promise<CommandOutcome> {
|
||||
this.loads++
|
||||
return this.run(async (at) => {
|
||||
const key = this.key
|
||||
@@ -371,9 +371,14 @@ class Entry<V> {
|
||||
} finally {
|
||||
this.loads--
|
||||
}
|
||||
// A conflicted key holds a value that is on screen only: the server refused it and
|
||||
// nothing can send it, so a read nobody asked for must not put the server's value in
|
||||
// its place, nor answer the conflict on the user's behalf. Checked here rather than
|
||||
// before the read, because the flush that precedes one is itself a way to find out.
|
||||
const standOff = unasked && this.ports.conflicted(key)
|
||||
// Whatever landed since the read was asked for — an external write, an edit — is
|
||||
// newer than what it read, so the read only moves the deployed side under it.
|
||||
const keepValue = this.edits !== at.edits
|
||||
const keepValue = standOff || this.edits !== at.edits
|
||||
this.meta = res.meta
|
||||
this.error = undefined
|
||||
if (isTemporaryPath(key.path)) {
|
||||
@@ -385,7 +390,9 @@ class Entry<V> {
|
||||
}
|
||||
this.serverDeployed = snapshot(res.deployed)
|
||||
this.origin = res.deployed !== undefined ? 'deployed' : 'draft'
|
||||
this.adoptRow(key, res.draft, res.draftSavedAt, at.rows)
|
||||
// The deployed side above always advances: the item did change, and a discard after
|
||||
// the user keeps theirs has to land on what the server holds now.
|
||||
if (!standOff) this.adoptRow(key, res.draft, res.draftSavedAt, at.rows)
|
||||
this.loaded = true
|
||||
if (!keepValue) {
|
||||
this.pristine = this.settles && this.origin === 'deployed' && res.draft === undefined
|
||||
@@ -401,15 +408,11 @@ class Entry<V> {
|
||||
* so they come back over what was written; one typed since the read was asked for stays. */
|
||||
async reread(): Promise<CommandOutcome> {
|
||||
if (!this.loaded || this.retired || !this.adapter) return { ok: true }
|
||||
// This re-read only holds because what is on screen comes back from its own row. A
|
||||
// conflicted key has no row of ours on the server and no way to put one there, so reading
|
||||
// would replace the value with the other writer's and call the conflict resolved.
|
||||
if (this.ports.conflicted(this.key)) return { ok: true }
|
||||
// Taken before the row goes, not after it lands: an edit typed while that write is in
|
||||
// flight is in neither it nor the read that follows, and is newer than both.
|
||||
const asOf = this.asOf()
|
||||
await this.ports.flush(this.key)
|
||||
return this.load(this.adapter, asOf)
|
||||
return this.load(this.adapter, asOf, true)
|
||||
}
|
||||
|
||||
/** An outside write (the AI chat, another editor): a real divergence, never settling. */
|
||||
|
||||
@@ -28,6 +28,8 @@ function fakeRows() {
|
||||
const conflicts = new Set<string>()
|
||||
/** Paths whose next write the server fails. */
|
||||
const failing = new Set<string>()
|
||||
/** Paths the server rejects when their queued row is actually sent. */
|
||||
const conflictOnFlush = new Set<string>()
|
||||
const failures = new Map<string, string>()
|
||||
const hints = new Map<string, boolean>()
|
||||
/** Paths whose parked payload was dropped. */
|
||||
@@ -55,6 +57,11 @@ function fakeRows() {
|
||||
const going = queued.has(key.path) ? { value: queued.get(key.path) } : undefined
|
||||
queued.delete(key.path)
|
||||
if (holdFlush) await holdFlush
|
||||
if (going && conflictOnFlush.has(key.path)) {
|
||||
conflictOnFlush.delete(key.path)
|
||||
conflicts.add(key.path)
|
||||
return
|
||||
}
|
||||
if (going) sent.set(key.path, going.value)
|
||||
},
|
||||
overwrite: async (key, value) => {
|
||||
@@ -80,6 +87,7 @@ function fakeRows() {
|
||||
port,
|
||||
writes,
|
||||
conflicts,
|
||||
conflictOnFlush,
|
||||
failing,
|
||||
hints,
|
||||
dropped,
|
||||
@@ -1143,6 +1151,43 @@ describe('item store: conflicts', () => {
|
||||
// conflict resolved. It is the user's to resolve, so it stays.
|
||||
expect(open.value?.description).toBe('mine')
|
||||
expect(open.status).toBe('conflicted')
|
||||
// The deployed side still learns the write: keeping mine and then discarding has to land
|
||||
// on what the server holds now, not on the config it held before the panel wrote.
|
||||
expect(open.deployed).toEqual({ ...b, args: { a: 2 } })
|
||||
expect(await open.discard()).toEqual({ removed: false })
|
||||
expect(open.value).toEqual({ ...b, args: { a: 2 } })
|
||||
})
|
||||
|
||||
it('keeps a local edit the flush before a re-read found a conflict for', async () => {
|
||||
const rows = fakeRows()
|
||||
const store = createItemStore(rows.port)
|
||||
const b = { ...deployedRes, path: 'u/me/b' }
|
||||
let deployed = b
|
||||
const theirs = { ...b, description: 'the other tab' }
|
||||
const { handle: open } = store.acquire(
|
||||
{ workspace: 'w', kind: 'resource', path: 'u/me/b' },
|
||||
{ workspace: 'w', path: 'u/me/b' },
|
||||
adapter(async () => ({ deployed, draft: theirs }))
|
||||
)
|
||||
await settle()
|
||||
open.value = { ...b, description: 'mine' }
|
||||
// The row is queued, not yet refused: the conflict only surfaces when the flush sends it,
|
||||
// which is the flush the re-read does before reading.
|
||||
rows.conflictOnFlush.add('u/me/b')
|
||||
|
||||
const temporary = newItemPath()
|
||||
const { handle: panel } = store.acquire(
|
||||
{ workspace: 'w', kind: 'resource', path: temporary },
|
||||
{ workspace: 'w', path: temporary, template: b, standsFor: 'u/me/b' },
|
||||
adapter({}, async (ctx) => void (deployed = { ...ctx.value }))
|
||||
)
|
||||
await settle()
|
||||
panel.value = { ...b, args: { a: 2 } }
|
||||
expect(await panel.save()).toMatchObject({ ok: true, path: 'u/me/b' })
|
||||
|
||||
expect(open.status).toBe('conflicted')
|
||||
expect(open.value?.description).toBe('mine')
|
||||
expect(open.deployed).toEqual({ ...b, args: { a: 2 } })
|
||||
})
|
||||
|
||||
it('clears the conflict on a reload the user typed during', async () => {
|
||||
|
||||
@@ -532,17 +532,19 @@ export const UserDraftDbSyncer = {
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Seed the per-tab `last_sync` after an editor reads a draft from the
|
||||
* server. Pass the response's `draft_saved_at` so the next save sends a
|
||||
* matching `last_sync`; pass `undefined` when no draft existed (next
|
||||
* save omits `last_sync`, the backend's first-push branch).
|
||||
*/
|
||||
/** 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
|
||||
},
|
||||
|
||||
/**
|
||||
* Seed the per-tab `last_sync` after an editor reads a draft from the
|
||||
* server. Pass the response's `draft_saved_at` so the next save sends a
|
||||
* matching `last_sync`; pass `undefined` when no draft existed (next
|
||||
* save omits `last_sync`, the backend's first-push branch). Pass the
|
||||
* `sendsSoFar` taken before the read as `since`: a row handed over after
|
||||
* that puts the syncer ahead of this response, which is then ignored.
|
||||
*/
|
||||
recordRemoteSync(
|
||||
query: UserDraftLastSyncQuery,
|
||||
draftSavedAt: string | undefined,
|
||||
|
||||
Reference in New Issue
Block a user