From d3fb0600161b1cb82a750470ec4a53af46bd6f9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mats=20T=C3=B6rnberg?= Date: Sun, 1 Nov 2026 13:57:00 +0100 Subject: [PATCH] Fix self-inflicted edit conflicts: dialogs no longer pin a stale node version Properties, Raw JSON and AI proposals sent the version captured when the dialog opened. Any other edit to the node meanwhile (style chip, drag handle, inline text, child insert/move) bumped it, so the save 409'd against the user's own work. Save through ctrl.saveData instead: compare only the keys being written against the current node (a real conflict is a key someone else changed) and send no version. 409 toasts now show the server's reason. Co-Authored-By: Claude Sonnet 5.5 --- src/ai/tools.ts | 2 +- src/controller.ts | 27 ++++++++++++++++++++++++++- src/editors/builtin.tsx | 4 ++-- test/assistant.test.ts | 9 ++++++--- test/editor.test.tsx | 21 ++++++++++++++++++++- 5 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/ai/tools.ts b/src/ai/tools.ts index 4c463d7..f4ce836 100644 --- a/src/ai/tools.ts +++ b/src/ai/tools.ts @@ -201,5 +201,5 @@ export async function applyProposal(ctrl: Controller, p: Proposal): Promise, patch: Record) { + let fresh: CmsNode + try { + fresh = await this.node(id, true) + } catch (e) { + this.fail(e) + return null + } + const clash = Object.keys(patch).filter((k) => JSON.stringify(fresh.data?.[k] ?? null) !== JSON.stringify(base[k] ?? null)) + if (clash.length) { + this.toast('error', `Someone else changed ${clash.join(', ')} - reopen it and retry`) + return null + } + return this.setData(id, patch) + } + async insert(parent: string, type: string, opts: { index?: number; handle?: string; data?: Record } = {}) { const res = await this.apply([{ op: 'insert', parent, type, ...opts }]) const created = res?.results[0]?.node @@ -1111,7 +1136,7 @@ export class Controller { const msg = e instanceof EditError || e instanceof ApiError ? e.status === 409 - ? 'Someone else changed this node - reopen it and retry' + ? `Conflict: ${e.message} - reopen it and retry` : e.status === 401 || e.status === 403 ? `Not allowed (${e.status}): ${e.message}` : e.message diff --git a/src/editors/builtin.tsx b/src/editors/builtin.tsx index cdb6c6b..6a4291d 100644 --- a/src/editors/builtin.tsx +++ b/src/editors/builtin.tsx @@ -27,7 +27,7 @@ function Properties({ node, schema, ctrl, close }: EditorProps) { const changed = Object.keys(patch).length > 0 const save = async () => { setSaving(true) - const res = await ctrl.setData(node.id, patch, { version: node.version }) + const res = await ctrl.saveData(node.id, node.data ?? {}, patch) setSaving(false) if (res) close() } @@ -70,7 +70,7 @@ function RawJson({ node, ctrl, close }: EditorProps) { {Object.keys(patch).length > 0 && (
-
diff --git a/test/assistant.test.ts b/test/assistant.test.ts index 72ea609..d4a88dd 100644 --- a/test/assistant.test.ts +++ b/test/assistant.test.ts @@ -8,8 +8,10 @@ const node = { id: 'n1', dataType: 'hero', version: 3, data: { title: 'Hello wro function fakeCtrl() { const setData = vi.fn(async () => ({})) + const saveData = vi.fn(async () => ({})) return { setData, + saveData, ctrl: { store: { get: () => ({ selected: 'n1', blocks: [{ id: 'n1', type: 'hero', depth: 0 }] }) }, block: (id: string) => (id === 'n1' ? { id } : undefined), @@ -19,6 +21,7 @@ function fakeCtrl() { schema: async () => schema, labelFor: () => 'Hero', setData, + saveData, } as any, } } @@ -80,10 +83,10 @@ describe('runAgent + tools', () => { }) describe('applyProposal', () => { - it('rebuilds the nested patch from the fresh node and saves with its version', async () => { - const { ctrl, setData } = fakeCtrl() + it('rebuilds the nested patch from the fresh node and saves it through the key-level conflict check', async () => { + const { ctrl, saveData } = fakeCtrl() await applyProposal(ctrl, { id: 1, kind: 'set', nodeId: 'n1', label: 'Hero', path: 'links.0.text', before: 'a', after: 'b' }) - expect(setData).toHaveBeenCalledWith('n1', { links: [{ text: 'b' }] }, { version: 3 }) + expect(saveData).toHaveBeenCalledWith('n1', node.data, { links: [{ text: 'b' }] }) }) }) diff --git a/test/editor.test.tsx b/test/editor.test.tsx index ce7e2d7..fd6ec2c 100644 --- a/test/editor.test.tsx +++ b/test/editor.test.tsx @@ -100,7 +100,7 @@ describe('on-page editor against the mock CMS', () => { expect(h.cms.graph.node('s1').data).toMatchObject({ title: 'Hello', tone: 'dark', padded: true }) expect(byId('s1').querySelector('.sec__title')!.textContent).toBe('Hello') const edit = h.editor.ctrl.store.get().log.find((l) => l.url === '/api/edit')! - expect((edit.request as any).ops[0]).toMatchObject({ op: 'set', id: 's1', data: { title: 'Hello' }, version: 1 }) + expect((edit.request as any).ops[0]).toMatchObject({ op: 'set', id: 's1', data: { title: 'Hello' } }) }) it('array editors save through the dialog as a minimal diff, and invalid data blocks Save', async () => { @@ -126,6 +126,25 @@ describe('on-page editor against the mock CMS', () => { expect(Object.keys(op.ops[0].data)).toEqual(['links']) // companyName, usps, social... untouched }) + it('edits to OTHER fields of the node while the dialog is open do not conflict (the dialog does not pin a version)', async () => { + h = await setup() + await selectBlock('s1') + await h.press('Properties') + // style chip / drag handle / inline edit on the same node bump its version under the open dialog + await h.editor.ctrl.setData('s1', { tone: 'accent' }) + await h.editor.ctrl.setData('s1', { padded: false }) + const input = h.shadow.querySelector('[data-path="title"] input')! + const set = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + await act(async () => { + set.call(input, 'Mine') + input.dispatchEvent(new Event('input', { bubbles: true })) + }) + await act(async () => void h.shadow.querySelector('.cms-btn--primary')!.click()) + await h.settle() + expect(h.cms.graph.node('s1').data).toMatchObject({ title: 'Mine', tone: 'accent', padded: false }) + expect(h.editor.ctrl.store.get().toast?.kind).not.toBe('error') + }) + it('a stale version shows a conflict instead of overwriting', async () => { h = await setup() await selectBlock('s1')