starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-10-13 08:02:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: count rows, not seeds, and seed a mirror the live item took from

Browse Source 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-15 22:54:54 +02:00
co-authored by Claude Opus 5
parent d902638c71
commit e58ba737e4
3 changed files with 64 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/lib/itemStore.svelte.ts
+12 -18
View File
@@ -249,17 +249,14 @@ 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, and each row a caller says it wrote for one of them. A row left over after that is a
* row nobody here has the value of. */
/** Counts the rows this entry can account for: the ones it wrote itself, and the ones a caller
* says it wrote for a value handed in here. A row left over after that is a row nobody here
* has the value of. */
private values = 0
/** Whether the discard that last ran had anything to say about the key's row — it had one, or
* it is deliberately keeping the one a newer edit put there. Decided inside the command, so
* after whatever load was queued in front of it, not before. */
private ownsRow = false
/** Whether the last value handed in made this entry write a row of its own. Loading, it
* cannot: the caller's own row is then the only one, and is already counted. */
private seedWroteRow = false
private queue: Promise<unknown> = Promise.resolve()
private ports: ItemRowPort
private store: StoreInternals
@@ -283,7 +280,6 @@ 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)) {
@@ -314,6 +310,7 @@ class Entry<V> {
}
if (desired === this.row) return
this.row = desired
this.values++
this.ports.write(key, desired === null ? null : snapshot(this.value))
}
@@ -454,8 +451,7 @@ class Entry<V> {
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 =
standOff || this.edits !== at.edits || this.externals !== at.externals
const keepValue = standOff || this.edits !== at.edits || this.externals !== at.externals
// 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
@@ -577,26 +573,24 @@ class Entry<V> {
})
}
/** An outside write (the AI chat, another editor): a real divergence, never settling. */
/** A caller that handed a value in through `applyExternal` and then wrote its own row for it,
* rather than leaving the row to this entry: the two are one value, and counting the second
* row as unaccounted would read as someone else having written it. */
/** A caller that handed a value in through `applyExternal` and wrote its own row for it,
* rather than leaving that to this entry: the row is accounted for, and counting it as
* unaccounted would read as someone else having written it. */
noteRow(): void {
if (this.seedWroteRow) this.values++
this.values++
}
/** An outside write (the AI chat, another editor): a real divergence, never settling. */
applyExternal(value: V): number {
this.externals++
this.values++
this.seedWroteRow = false
if (this.loaded && serialize(value) === serialize(this.value)) return this.revision
this.edits++
this.pristine = false
this.removed = false
this.replaceValue(value)
const rowsBefore = this.ports.rowMark(this.key)
// Whatever row this produces is counted by `reconcile`; a value that produces none
// accounts for nothing, and must not pay for a row some other writer puts there.
this.reconcile()
this.seedWroteRow = this.ports.rowMark(this.key) !== rowsBefore
return this.revision
}
frontend/src/lib/itemStore.test.ts
+34 -4
View File
@@ -102,8 +102,7 @@ function fakeRows() {
: parked.has(key.path)
? parked.get(key.path)
: undefined,
unbased: (key) =>
(queued.has(key.path) || parked.has(key.path)) && !baselines.has(key.path),
unbased: (key) => (queued.has(key.path) || parked.has(key.path)) && !baselines.has(key.path),
markConflict: (key) => void conflicts.add(key.path),
seedSync: (key, at, since) => {
// The syncer refuses a baseline from a read a row handed over since has passed.
@@ -734,7 +733,8 @@ describe('item store: origins', () => {
// The chat writes after asking for the discard, so its value is the newer of the two.
const fromChat = { ...deployedRes, description: 'from the chat' }
item.applyExternal(fromChat)
// `persistGlobalDraft` hands the value here and writes its own row through the syncer.
// `persistGlobalDraft` hands the value here, says it is writing the row for it, and does.
store.bridge.noteRow('w', 'resource', 'u/me/r')
rows.handExternally('u/me/r')
read.resolve({ deployed: deployedRes })
@@ -882,6 +882,7 @@ describe('item store: origins', () => {
const fromChat = { ...deployedRes, description: 'from the chat' }
item.applyExternal(fromChat)
// `persistGlobalDraft` hands the value here and writes its own row through the syncer.
store.bridge.noteRow('w', 'resource', 'u/me/r')
rows.handExternally('u/me/r')
read.resolve({ deployed: deployedRes })
await settle()
@@ -951,7 +952,7 @@ describe('item store: origins', () => {
expect(item.canSave).toBe(false)
})
it('does not credit an edit made during a re-read\'s own flush', async () => {
it("does not credit an edit made during a re-read's own flush", async () => {
const rows = fakeRows()
const flushing = deferred()
const read = deferred<ItemLoad<Res>>()
@@ -1009,6 +1010,35 @@ describe('item store: origins', () => {
expect(item.canSave).toBe(false)
})
it('does not credit a row to a seed that wrote none', async () => {
const rows = fakeRows()
const read = deferred<ItemLoad<Res>>()
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(() => read.promise)
)
await settle()
// The agent editor seeds its restored draft here while the GET is out. Loading, this
// writes no row, and the seeding editor writes none for it either.
const seeded = { ...deployedRes, description: 'restored by the agent editor' }
item.applyExternal(seeded)
// It then autosaves something newer through the `UserDraft` handle it kept: a row whose
// value never came through here.
rows.handExternally('u/me/r')
const writesBefore = rows.writes.length
read.resolve({ deployed: deployedRes })
await settle()
// One row, nothing accounting for it. Posting the seed now would put it on the baseline
// that row advanced, silently replacing it.
expect(item.canSave).toBe(false)
expect(rows.writes.length).toBe(writesBefore)
})
it('stays blocked when an unseen row follows the edit it kept', async () => {
const rows = fakeRows()
const read = deferred<ItemLoad<Res>>()
frontend/src/lib/userDraft.svelte.ts
+18 -7
View File
@@ -333,10 +333,25 @@ export type UserDraftHandle<V> = {
set draft(value: V | undefined)
}
/**
* Put a value into a caller's own mirror without POSTing it, for a key whose live item took the
* value and persists it. A mirror left untouched keeps the first-write guard it was acquired with
* armed, so the caller's next real edit is swallowed as initialization instead of being saved.
*/
function seedLocalMirror(mk: string, value: unknown): void {
const entry = entries.get(mk)
if (!entry) return
entry.seedNextWrite = true
entry.state.val = snapshotDraftValue(value)
}
export const UserDraft = {
save<V>(itemKind: UserDraftItemKind, path: string, value: V, opts?: UserDraftOptions): void {
const ws = resolveWorkspace(opts)
if (liveItems?.seed(ws, itemKind, path, value)) return
if (liveItems?.seed(ws, itemKind, path, value)) {
seedLocalMirror(mapKey(ws, itemKind, path), value)
return
}
const mk = mapKey(ws, itemKind, path)
const entry = entries.get(mk)
if (entry) {
@@ -525,12 +540,8 @@ export const UserDraft = {
*/
seed<V>(itemKind: UserDraftItemKind, path: string, value: V, opts?: UserDraftOptions): void {
const ws = resolveWorkspace(opts)
if (liveItems?.seed(ws, itemKind, path, value)) return
const mk = mapKey(ws, itemKind, path)
const entry = entries.get(mk)
if (!entry) return
entry.seedNextWrite = true
entry.state.val = snapshotDraftValue(value)
liveItems?.seed(ws, itemKind, path, value)
seedLocalMirror(mapKey(ws, itemKind, path), value)
},
/**