mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-14 00:02:55 +00:00
fix: weigh rows appearing during a read against the values that account for them
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f753d993ff
commit
45add0a4c0
@@ -159,8 +159,9 @@ const outOfDate = {
|
||||
/** What a command did not do, so it can tell what moved under it. `edits`: what the user can see
|
||||
* change, which a discard measures against, so an outside write of the value already shown does
|
||||
* not cancel one. `externals`: every outside write, that one included — it still made the value
|
||||
* a row, which a read in flight has not got and must not answer over. */
|
||||
type AsOf = { edits: number; externals: number }
|
||||
* a row, which a read in flight has not got and must not answer over. `values`: both of those,
|
||||
* counted once each, to weigh against the rows that appeared alongside them. */
|
||||
type AsOf = { edits: number; externals: number; values: number }
|
||||
|
||||
class Entry<V> {
|
||||
key: ItemKey = $state()!
|
||||
@@ -245,6 +246,9 @@ class Entry<V> {
|
||||
/** Counts outside writes, the ones that change nothing on screen included: those still put the
|
||||
* value on the server as a row, which a read already in flight knows nothing about. */
|
||||
private externals = 0
|
||||
/** Counts every value this entry took from outside a command — typed or written in — once
|
||||
* each. A row that appeared without one of these is a row nobody here has the value of. */
|
||||
private values = 0
|
||||
private queue: Promise<unknown> = Promise.resolve()
|
||||
private ports: ItemRowPort
|
||||
private store: StoreInternals
|
||||
@@ -268,6 +272,7 @@ class Entry<V> {
|
||||
if (s === this.seen) return
|
||||
this.seen = s
|
||||
this.edits++
|
||||
this.values++
|
||||
if (this.pristine && this.origin === 'deployed' && this.value !== undefined) {
|
||||
const value = snapshot(this.value)
|
||||
if (!this.absorbs || this.deployed === undefined || this.absorbs(value, this.deployed)) {
|
||||
@@ -365,7 +370,7 @@ class Entry<V> {
|
||||
/** The registers that move on their own: what the user is editing, and the rows already sent.
|
||||
* A command compares against these to tell what has happened since it was asked for. */
|
||||
private asOf(): AsOf {
|
||||
return { edits: this.edits, externals: this.externals }
|
||||
return { edits: this.edits, externals: this.externals, values: this.values }
|
||||
}
|
||||
|
||||
/** Count a command as started now, ahead of its turn in the queue; returns its release. */
|
||||
@@ -419,11 +424,12 @@ class Entry<V> {
|
||||
// newer than what it read, so the read only moves the deployed side under it.
|
||||
const keepValue =
|
||||
standOff || this.edits !== at.edits || this.externals !== at.externals
|
||||
// A row landed while the read was out and no newer value came with it, so it was
|
||||
// written by someone this entry cannot see. Its response predates that row, and
|
||||
// writing from it would post it back over it on the baseline that row advanced — so
|
||||
// nothing goes out, and nothing is saved or discarded, until a reload.
|
||||
this.stale = this.ports.rowMark(key) !== rowsAtRead && !keepValue
|
||||
// More rows appeared while the read was out than values arrived to account for them,
|
||||
// so one was written by someone this entry cannot see — and it is later than anything
|
||||
// held here. Writing from this would post an older value over that row, on the
|
||||
// baseline it advanced, so nothing goes out and nothing is saved until a reload.
|
||||
this.stale =
|
||||
this.ports.rowMark(key) - rowsAtRead > this.values - at.values
|
||||
this.meta = res.meta
|
||||
this.error = undefined
|
||||
if (isTemporaryPath(key.path)) {
|
||||
@@ -520,6 +526,7 @@ class Entry<V> {
|
||||
/** An outside write (the AI chat, another editor): a real divergence, never settling. */
|
||||
applyExternal(value: V): number {
|
||||
this.externals++
|
||||
this.values++
|
||||
if (this.loaded && serialize(value) === serialize(this.value)) return this.revision
|
||||
this.edits++
|
||||
this.pristine = false
|
||||
|
||||
@@ -873,6 +873,34 @@ describe('item store: origins', () => {
|
||||
expect(rows.writes.at(-1)).toEqual({ path: 'u/me/r', value: typed })
|
||||
})
|
||||
|
||||
it('stays blocked when an unseen row follows the edit it kept', async () => {
|
||||
const rows = fakeRows()
|
||||
const read = deferred<ItemLoad<Res>>()
|
||||
let reads = 0
|
||||
const store = createItemStore(rows.port)
|
||||
const key: ItemKey = { workspace: 'w', kind: 'resource', path: 'u/me/r' }
|
||||
const { handle: item } = store.acquire(
|
||||
key,
|
||||
{ workspace: 'w', path: 'u/me/r' },
|
||||
adapter(() => (reads++ === 0 ? Promise.resolve({ deployed: deployedRes }) : read.promise))
|
||||
)
|
||||
await settle()
|
||||
|
||||
const reloading = item.reload()
|
||||
await settle()
|
||||
// The user types while the GET is out: newer than the read, and its own row goes out.
|
||||
item.value = { ...deployedRes, description: 'mine' }
|
||||
// Then another draft surface writes this key directly, later than that edit.
|
||||
rows.handExternally('u/me/r')
|
||||
read.resolve({ deployed: deployedRes })
|
||||
await reloading
|
||||
|
||||
// A value did arrive during the read, but the unseen row came after it, so what is held
|
||||
// here is older than what the server has. Saving it would deploy the older of the two.
|
||||
expect(item.value?.description).toBe('mine')
|
||||
expect(item.canSave).toBe(false)
|
||||
})
|
||||
|
||||
it('does not post a read back over a row another editor landed during it', async () => {
|
||||
const rows = fakeRows()
|
||||
const read = deferred<ItemLoad<Res>>()
|
||||
|
||||
Reference in New Issue
Block a user