diff --git a/.changeset/11773-studio-draft-save-if-match.md b/.changeset/11773-studio-draft-save-if-match.md new file mode 100644 index 0000000000..750e98bcce --- /dev/null +++ b/.changeset/11773-studio-draft-save-if-match.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +--- + +Studio's metadata draft saves send the version they were built on (objectui#11773). Once an editor has saved, its later saves no longer replace a draft that was saved elsewhere in the meantime without saying so. + +Each guarded draft save of an existing item now sends `If-Match` with the `version` the editor's previous save received, and records the new receipt's version. The `/meta` draft door refuses a stale version with `409 METADATA_CONFLICT`. The editor then shows a dialog with three choices: reload the saved version (unsaved edits on screen are dropped), overwrite it after a second confirmation (the save is sent again without `If-Match`), or keep editing (nothing is saved, and the next save is refused again). The destructive-change confirmation (`409 DESTRUCTIVE_CHANGE`) is a separate flow and is unchanged. + +The guarded saves are the Data pillar's object autosave and column reorder, the Automations pillar's flow autosave and enable switch, the Interfaces pillar's autosave of the open item (a page, dashboard or other item it edits) and its navigation autosave, the metadata designer's draft save, the Access pillar's package-scoped permission-set save, and the object Hooks panel's save. Creating an item sends no `If-Match`. + +The protection starts at an editor's second save. A draft read serves no version, so the first save after an editor loads, reloads or switches items is still sent without `If-Match`. + +Nothing on the package entry changes. The guard (`useDraftSaveGuard`) and its dialog are not exported from `@object-ui/app-shell`, and the new strings are rows in the designer's module-local string table, not language-pack keys. diff --git a/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.test.tsx b/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.test.tsx new file mode 100644 index 0000000000..e3af8b0171 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.test.tsx @@ -0,0 +1,302 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — the draft-save version guard, against a door double that + * answers the way the `/meta` PUT door of `@objectstack/*` 17.7.0 was MEASURED + * to answer (the readings are on the pull request): + * + * - a `?mode=draft` save returns `{ success, version, seq, state, message }`; + * `version` is the token, keyed (`hmac-sha256:…`); + * - an `If-Match` that is not the current draft's token — including any token + * while no draft row exists — is `409 { error, code: 'METADATA_CONFLICT' }`, + * with the current token only inside the prose; + * - a destructive change is `409 { error, code: 'DESTRUCTIVE_CHANGE', issues }` + * unless `?force=true`, judged BEFORE the version. + * + * The client is the REAL `MetadataClient`, so the header on the wire and the + * parsed refusal are production's, not a hand-built error object. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { MetadataClient } from '@object-ui/data-objectstack'; +import { + DraftVersionGuard, + isDraftVersionConflict, + type DraftConflictChoice, +} from './DraftConflictDialog'; + +interface SentPut { + type: string; + name: string; + draft: boolean; + force: boolean; + ifMatch: string | null; + body: Record; +} + +function json(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); +} + +/** The measured draft-write door, over `fetch`. */ +function measuredDoor() { + const rows = new Map; version: string }>(); + const puts: SentPut[] = []; + let seq = 0; + const rowKey = (type: string, name: string, pkg: string | null, draft: boolean) => + `${draft ? 'draft' : 'active'}:${type}/${name}@${pkg ?? ''}`; + const write = (key: string, body: Record) => { + seq += 1; + const version = `hmac-sha256:${seq.toString(16).padStart(64, '0')}`; + rows.set(key, { body, version }); + return version; + }; + const fetchImpl = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = new URL(String(input)); + const [, , , , type = '', name = ''] = url.pathname.split('/').map(decodeURIComponent); + if (init?.method !== 'PUT') return json(404, { error: 'not modelled' }); + const draft = url.searchParams.get('mode') === 'draft'; + const force = url.searchParams.get('force') === 'true'; + const ifMatch = new Headers(init.headers).get('If-Match'); + const body = JSON.parse(String(init.body)) as Record; + puts.push({ type, name, draft, force, ifMatch, body }); + if (body.dropsAField && !force) { + return json(409, { + error: `${type}/${name} would drop or transform existing data: Field 'subject' removed. — re-submit with ?force=true to proceed.`, + code: 'DESTRUCTIVE_CHANGE', + issues: [{ code: 'field_removed', field: 'subject', message: "Field 'subject' removed." }], + }); + } + const key = rowKey(type, name, url.searchParams.get('package'), draft); + const head = rows.get(key)?.version ?? null; + if (ifMatch !== null && (head === null || ifMatch.replace(/^"|"$/g, '') !== head)) { + return json(409, { + error: `${type}/${name} has been modified since you loaded it. The version token sent is not the current version (current is ${head}).`, + code: 'METADATA_CONFLICT', + }); + } + const version = write(key, body); + return json(200, { + success: true, + version, + seq, + state: draft ? 'draft' : 'active', + message: `Saved ${type} '${name}' [seq=${seq}]`, + }); + }) as typeof fetch; + return { + client: new MetadataClient({ baseUrl: 'http://localhost:3000', fetch: fetchImpl }), + puts, + /** Another editor's save of the same draft row, outside this guard. */ + savedElsewhere(type: string, name: string, pkg: string, body: Record) { + write(rowKey(type, name, pkg, true), body); + }, + draftBody(type: string, name: string, pkg: string) { + return rows.get(rowKey(type, name, pkg, true))?.body; + }, + /** A publish promotes the draft row and drops it (`current is null` after). */ + publishDraft(type: string, name: string, pkg: string) { + const key = rowKey(type, name, pkg, true); + const row = rows.get(key); + if (row) write(rowKey(type, name, pkg, false), row.body); + rows.delete(key); + }, + }; +} + +function guardOver(door: ReturnType, answer: DraftConflictChoice = 'cancel') { + const ask = vi.fn(async () => answer); + const reload = vi.fn(); + const guard = new DraftVersionGuard({ + client: () => door.client, + ask, + reload, + notSaved: (c) => new Error(`not saved: ${c.type}/${c.name}`), + }); + return { guard, ask, reload }; +} + +const DRAFT = { mode: 'draft' as const, packageId: 'com.acme.app' }; + +describe('DraftVersionGuard — each draft save sends the version the last one received (objectui#11773)', () => { + it('a first save sends no If-Match; every later save sends the previous receipt\'s version', async () => { + const door = measuredDoor(); + const { guard, ask } = guardOver(door); + expect(await guard.save('object', 'acme_task', { label: 'A' }, DRAFT)).toBe('saved'); + expect(await guard.save('object', 'acme_task', { label: 'B' }, DRAFT)).toBe('saved'); + expect(await guard.save('object', 'acme_task', { label: 'C' }, DRAFT)).toBe('saved'); + expect(door.puts.map((p) => p.ifMatch)).toEqual([ + null, + `hmac-sha256:${'1'.padStart(64, '0')}`, + `hmac-sha256:${'2'.padStart(64, '0')}`, + ]); + expect(ask).not.toHaveBeenCalled(); + }); + + it('saves sent back to back go one at a time, so the second never conflicts with the first', async () => { + const door = measuredDoor(); + const { guard, ask } = guardOver(door); + await guard.save('object', 'acme_task', { label: 'A' }, DRAFT); + // An autosave and an explicit save (a column reorder) fired together. + const [a, b] = await Promise.all([ + guard.save('object', 'acme_task', { label: 'B' }, DRAFT), + guard.save('object', 'acme_task', { label: 'C' }, DRAFT), + ]); + expect([a, b]).toEqual(['saved', 'saved']); + expect(ask).not.toHaveBeenCalled(); + expect(door.puts[2]!.ifMatch).toBe(`hmac-sha256:${'2'.padStart(64, '0')}`); + expect(door.draftBody('object', 'acme_task', 'com.acme.app')).toEqual({ label: 'C' }); + }); + + it('forget(): a buffer installed from a read holds no version, so its next save is unpinned', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door); + await guard.save('object', 'acme_task', { label: 'A' }, DRAFT); + guard.forget(); + await guard.save('object', 'acme_task', { label: 'B' }, DRAFT); + expect(door.puts.map((p) => p.ifMatch)).toEqual([null, null]); + }); + + it('a version belongs to one item: a save of another item (or package) sends none', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door); + await guard.save('object', 'acme_task', { label: 'A' }, DRAFT); + await guard.save('object', 'acme_note', { label: 'N' }, DRAFT); + await guard.save('object', 'acme_task', { label: 'B' }, { mode: 'draft', packageId: 'com.acme.other' }); + expect(door.puts.map((p) => p.ifMatch)).toEqual([null, null, null]); + }); + + it('a non-draft save passes straight through, unpinned and unrecorded', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door); + await guard.save('permission', 'sales', { a: 1 }, {}); + await guard.save('permission', 'sales', { a: 2 }, {}); + expect(door.puts.map((p) => [p.draft, p.ifMatch])).toEqual([ + [false, null], + [false, null], + ]); + }); +}); + +describe('DraftVersionGuard — a draft saved elsewhere is not overwritten in silence (objectui#11773)', () => { + it('the stale save is refused, nothing else is sent, and "reload" hands the buffer back to the caller', async () => { + const door = measuredDoor(); + const { guard, ask, reload } = guardOver(door, 'reload'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + door.savedElsewhere('object', 'acme_task', 'com.acme.app', { label: 'mine', description: 'theirs' }); + + expect(await guard.save('object', 'acme_task', { label: 'mine, again' }, DRAFT)).toBe('reloaded'); + expect(ask).toHaveBeenCalledWith({ type: 'object', name: 'acme_task' }); + expect(reload).toHaveBeenCalledTimes(1); + // Their field survives: the refused body was never written. + expect(door.draftBody('object', 'acme_task', 'com.acme.app')).toEqual({ label: 'mine', description: 'theirs' }); + expect(door.puts).toHaveLength(2); + + // The reloaded buffer was read, not saved: its first save is unpinned. + await guard.save('object', 'acme_task', { label: 'mine', description: 'theirs', icon: 'x' }, DRAFT); + expect(door.puts[2]!.ifMatch).toBeNull(); + }); + + it('a save queued behind the refused one carries the replaced buffer and is dropped on "reload"', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door, 'reload'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + door.savedElsewhere('object', 'acme_task', 'com.acme.app', { label: 'theirs' }); + const [first, queued] = await Promise.all([ + guard.save('object', 'acme_task', { label: 'stale 1' }, DRAFT), + guard.save('object', 'acme_task', { label: 'stale 2' }, DRAFT), + ]); + expect([first, queued]).toEqual(['reloaded', 'reloaded']); + expect(door.puts).toHaveLength(2); + expect(door.draftBody('object', 'acme_task', 'com.acme.app')).toEqual({ label: 'theirs' }); + }); + + it('"overwrite" re-sends the same body without If-Match, wins, and pins the next save to its receipt', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door, 'overwrite'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + door.savedElsewhere('object', 'acme_task', 'com.acme.app', { label: 'theirs' }); + + expect(await guard.save('object', 'acme_task', { label: 'mine, kept' }, DRAFT)).toBe('saved'); + expect(door.puts.slice(1).map((p) => [p.ifMatch === null, p.body])).toEqual([ + [false, { label: 'mine, kept' }], + [true, { label: 'mine, kept' }], + ]); + expect(door.draftBody('object', 'acme_task', 'com.acme.app')).toEqual({ label: 'mine, kept' }); + + await guard.save('object', 'acme_task', { label: 'next' }, DRAFT); + // seq 1 mine, seq 2 theirs, seq 3 the overwrite. + expect(door.puts[3]!.ifMatch).toBe(`hmac-sha256:${'3'.padStart(64, '0')}`); + }); + + it('"keep editing" sends nothing, rejects, and keeps the stale version so the next save is refused again', async () => { + const door = measuredDoor(); + const { guard, ask } = guardOver(door, 'cancel'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + door.savedElsewhere('object', 'acme_task', 'com.acme.app', { label: 'theirs' }); + + await expect(guard.save('object', 'acme_task', { label: 'stale' }, DRAFT)).rejects.toThrow( + 'not saved: object/acme_task', + ); + await expect(guard.save('object', 'acme_task', { label: 'still stale' }, DRAFT)).rejects.toThrow( + 'not saved: object/acme_task', + ); + expect(ask).toHaveBeenCalledTimes(2); + expect(door.puts.slice(1).every((p) => p.ifMatch === `hmac-sha256:${'1'.padStart(64, '0')}`)).toBe(true); + expect(door.draftBody('object', 'acme_task', 'com.acme.app')).toEqual({ label: 'theirs' }); + }); + + it('control: once a publish dropped the draft, ANY version is refused — which is why an install must forget()', async () => { + const door = measuredDoor(); + const { guard, ask } = guardOver(door, 'cancel'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + door.publishDraft('object', 'acme_task', 'com.acme.app'); + + await expect(guard.save('object', 'acme_task', { label: 'after publish' }, DRAFT)).rejects.toThrow('not saved'); + expect(ask).toHaveBeenCalledTimes(1); + // What every pillar does when the publish reload installs the buffer. + guard.forget(); + expect(await guard.save('object', 'acme_task', { label: 'after publish' }, DRAFT)).toBe('saved'); + expect(door.puts.map((p) => p.ifMatch === null)).toEqual([true, false, true]); + }); +}); + +describe('DraftVersionGuard — the door\'s two 409s stay apart (objectui#11773)', () => { + it('DESTRUCTIVE_CHANGE passes through untouched, and the forced retry carries the same If-Match', async () => { + const door = measuredDoor(); + const { guard, ask } = guardOver(door, 'overwrite'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + + const refusal = await guard + .save('object', 'acme_task', { label: 'mine', dropsAField: true }, DRAFT) + .catch((e: unknown) => e); + expect(refusal).toMatchObject({ status: 409, code: 'DESTRUCTIVE_CHANGE' }); + expect(isDraftVersionConflict(refusal)).toBe(false); + expect(ask).not.toHaveBeenCalled(); + + // The caller's own confirmation flow re-sends with `force`. + expect(await guard.save('object', 'acme_task', { label: 'mine', dropsAField: true }, { ...DRAFT, force: true })).toBe( + 'saved', + ); + const pinned = `hmac-sha256:${'1'.padStart(64, '0')}`; + expect(door.puts.slice(1).map((p) => [p.force, p.ifMatch])).toEqual([ + [false, pinned], + [true, pinned], + ]); + }); + + it('isDraftVersionConflict reads the code on the parsed refusal, never the prose', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door, 'cancel'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + door.savedElsewhere('object', 'acme_task', 'com.acme.app', { label: 'theirs' }); + const raw = await door.client + .save('object', 'acme_task', { label: 'x' }, { ...DRAFT, ifMatch: 'hmac-sha256:stale' }) + .catch((e: unknown) => e); + expect(raw).toMatchObject({ status: 409, code: 'METADATA_CONFLICT' }); + expect(isDraftVersionConflict(raw)).toBe(true); + expect(isDraftVersionConflict(Object.assign(new Error('has been modified since you loaded it'), { status: 409 }))).toBe( + false, + ); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.tsx b/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.tsx new file mode 100644 index 0000000000..866eec3fa7 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.tsx @@ -0,0 +1,394 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — optimistic concurrency for Studio's metadata DRAFT saves. + * + * Two editors of one metadata item (two admins, or one admin in two tabs) used + * to overwrite each other's drafts in silence: every draft save was a + * whole-document last-writer-wins PUT, because no save sent the version it was + * built on. The `/meta` PUT door honours `If-Match` on a `?mode=draft` write + * (ADR-0008's `parentVersion`), and `MetadataClient.save` already sends its + * `ifMatch` option as that header; nothing in app-shell passed one. + * + * ## Where the version comes from (measured, not read off a docblock) + * + * Against a running `@objectstack/*` 17.7.0 server — the release this repo + * resolves — the draft-write door answered as follows (the readings are on the + * pull request that landed this module): + * + * - A draft SAVE answers `{ success, version, seq, state, message }`, and + * `version` is the token: the server's keyed digest of the stored content + * hash. No `ETag` header. + * - A draft READ (`GET …?state=draft`) serves no token at all — neither a body + * key nor an `ETag`. So the first save after an editor installs a buffer + * from a read cannot be pinned; it is sent without `If-Match`, exactly as + * every save was before. That half is the server's to add. + * - A save whose `If-Match` is not the current draft's token is refused + * `409 METADATA_CONFLICT`, and that includes ANY token sent when no draft row + * exists (after a publish or a discard dropped it). + * - `409 DESTRUCTIVE_CHANGE` is a different refusal, judged BEFORE the version; + * it is passed through untouched to the caller's own flow. + * + * ## One guard per editing buffer + * + * The version a guard holds describes the buffer it saves, so a guard belongs + * to ONE buffer — never to an item globally. Two surfaces in one tab that each + * hold their own copy of the same item are two editors: sharing one version + * between them would let the second one's stale copy through. The rules: + * + * 1. A draft save sends `If-Match` = the version this guard holds for that + * same item (type, name, package), and nothing otherwise. + * 2. A save that lands holds the receipt's `version`. + * 3. A buffer installed from a server read (a load, an item switch, a reload + * after a publish or a discard) holds no version: the caller says so with + * `forget()`. A read-back of the guard's OWN save is not such an install. + * 4. A `409 METADATA_CONFLICT` opens the conflict dialog: reload the saved + * version (the caller's reload runs; the buffer is replaced), overwrite it + * after a confirmation (re-sent without `If-Match`: the refusal carries the + * current version only inside its prose, which is not read), or keep + * editing (nothing is saved, and the stale version is kept so the next save + * is refused again rather than slipping through). + * 5. Saves through one guard run one at a time, so each sends the version the + * previous one received and an autosave never conflicts with the save the + * same buffer sent a moment earlier. + * + * A create sends no `If-Match` — the door cannot express "expect no row" over + * HTTP — so callers that create keep calling the client directly. + */ + +import * as React from 'react'; +import { AlertTriangle } from 'lucide-react'; +import { + AlertDialog, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + buttonVariants, + cn, +} from '@object-ui/components'; +import type { MetadataClient, MetadataClientSaveOptions } from '@object-ui/data-objectstack'; +import { errorCodeIs } from '@object-ui/types'; +import { t, tFormat, translateMetadataType, useMetadataLocale } from './i18n.js'; + +/** What a guarded save ended in, when it did not throw. */ +export type DraftSaveOutcome = 'saved' | 'reloaded'; + +/** The author's answer to a conflict. */ +export type DraftConflictChoice = 'reload' | 'overwrite' | 'cancel'; + +/** The item a refused save addressed. */ +export interface DraftConflict { + type: string; + name: string; +} + +/** + * The version-conflict refusal of the `/meta` draft door, told apart from the + * door's OTHER 409 (`DESTRUCTIVE_CHANGE`) by its code, never by its prose. + */ +export function isDraftVersionConflict(err: unknown): boolean { + return (err as { status?: unknown } | null | undefined)?.status === 409 && errorCodeIs(err, 'METADATA_CONFLICT'); +} + +interface HeldVersion { + key: string; + version: string; +} + +function itemKey(type: string, name: string, packageId: string | null | undefined): string { + return JSON.stringify([type, name, packageId ?? null]); +} + +function receiptVersion(receipt: unknown): string | null { + const version = (receipt as { version?: unknown } | null | undefined)?.version; + return typeof version === 'string' && version.length > 0 ? version : null; +} + +export interface DraftVersionGuardDeps { + /** The client to save through, read at the moment of the save. */ + client: () => Pick; + /** Ask the author how to resolve a conflict. */ + ask: (conflict: DraftConflict) => Promise; + /** Replace the buffer with the saved version (the caller's load). */ + reload: () => void; + /** The error a save the author chose not to send ends in. */ + notSaved: (conflict: DraftConflict) => Error; +} + +/** + * The React-free half of the guard: the version held for one buffer, the + * one-at-a-time queue, and the conflict branch. {@link useDraftSaveGuard} binds + * it to a client, a dialog and the caller's reload. + */ +export class DraftVersionGuard { + private held: HeldVersion | null = null; + private tail: Promise | null = null; + /** Bumped by a reload: a save queued before it carries the replaced buffer. */ + private epoch = 0; + + constructor(private deps: DraftVersionGuardDeps) {} + + /** Re-bind inputs that change between renders (the client, the reload, the locale's text). */ + bind(deps: Pick): void { + this.deps = { ...this.deps, ...deps }; + } + + /** The version held, for the item `save` would address (tests read it). */ + heldFor(type: string, name: string, packageId?: string | null): string | null { + return this.held?.key === itemKey(type, name, packageId) ? this.held.version : null; + } + + /** The buffer was installed from a server read: no version describes it. */ + forget = (): void => { + this.held = null; + }; + + /** + * Save `item` through the client. A non-draft save (no `mode: 'draft'`) is + * passed straight through: the guard pins drafts only. + */ + save = ( + type: string, + name: string, + item: unknown, + options: MetadataClientSaveOptions = {}, + ): Promise => { + if (options.mode !== 'draft') { + return this.deps.client().save(type, name, item, options).then(() => 'saved' as const); + } + const epoch = this.epoch; + const run = (): Promise => this.run(epoch, type, name, item, options); + // Started at once when nothing is in flight, so the request leaves in the + // same tick as before; queued behind the save in flight otherwise. + const result = this.tail ? this.tail.then(run) : run(); + const settled = result.then( + () => undefined, + () => undefined, + ); + this.tail = settled; + void settled.then(() => { + if (this.tail === settled) this.tail = null; + }); + return result; + }; + + private async run( + epoch: number, + type: string, + name: string, + item: unknown, + options: MetadataClientSaveOptions, + ): Promise { + // The author chose to reload while this save waited: its buffer is gone. + if (epoch !== this.epoch) return 'reloaded'; + const key = itemKey(type, name, options.packageId); + const pinned = this.held?.key === key ? this.held.version : null; + const client = this.deps.client(); + try { + const receipt = await client.save(type, name, item, pinned ? { ...options, ifMatch: pinned } : options); + this.hold(key, receipt); + return 'saved'; + } catch (err) { + if (!isDraftVersionConflict(err)) throw err; + const conflict: DraftConflict = { type, name }; + const choice = await this.deps.ask(conflict); + if (choice === 'reload') { + this.held = null; + this.epoch += 1; + this.deps.reload(); + return 'reloaded'; + } + if (choice === 'overwrite') { + // Unpinned from here on: the author has seen that the draft moved and + // chose this buffer over it, so a refusal of the re-send for another + // reason (a destructive change to confirm) retries unpinned too. + this.held = null; + const receipt = await client.save(type, name, item, options); + this.hold(key, receipt); + return 'saved'; + } + throw this.deps.notSaved(conflict); + } + } + + private hold(key: string, receipt: unknown): void { + const version = receiptVersion(receipt); + this.held = version ? { key, version } : null; + } +} + +export interface DraftSaveGuard { + /** A guarded `client.save` — same arguments, resolves what it ended in. */ + save: DraftVersionGuard['save']; + /** Call where the buffer is installed from a server read (rule 3 above). */ + forget: () => void; + /** The conflict dialog; render it once, anywhere in the caller's tree. */ + dialog: React.ReactElement; +} + +/** + * One guard for one editing buffer. `onReload` replaces the buffer with the + * saved version — the caller's own load, re-run. + */ +export function useDraftSaveGuard(client: Pick, onReload: () => void): DraftSaveGuard { + const locale = useMetadataLocale(); + const [pending, setPending] = React.useState<{ + conflict: DraftConflict; + resolve: (choice: DraftConflictChoice) => void; + } | null>(null); + const notSavedIn = (lang: string) => (conflict: DraftConflict) => + new Error( + tFormat('engine.draftConflict.notSaved', lang, { + type: translateMetadataType(conflict.type, lang), + name: conflict.name, + }), + ); + // A state initializer, not a memo: the guard and the version it holds must + // outlive any render (AGENTS.md #10). Its per-render inputs are re-bound + // below, after every commit. + const [guard] = React.useState( + () => + new DraftVersionGuard({ + client: () => client, + ask: (conflict) => + new Promise((resolve) => { + setPending({ conflict, resolve }); + }), + reload: onReload, + notSaved: notSavedIn(locale), + }), + ); + React.useLayoutEffect(() => { + guard.bind({ client: () => client, reload: onReload, notSaved: notSavedIn(locale) }); + }); + // A dialog left open by an unmount answers "keep editing": nothing is sent. + const pendingRef = React.useRef(pending); + React.useLayoutEffect(() => { + pendingRef.current = pending; + }); + React.useEffect(() => () => pendingRef.current?.resolve('cancel'), []); + const choose = React.useCallback((choice: DraftConflictChoice) => { + const asked = pendingRef.current; + setPending(null); + asked?.resolve(choice); + }, []); + return { + save: guard.save, + forget: guard.forget, + dialog: , + }; +} + +export interface DraftConflictDialogProps { + /** The refused save's item; `null` keeps the dialog closed. */ + conflict: DraftConflict | null; + onChoose: (choice: DraftConflictChoice) => void; +} + +/** + * Tells the author their draft save was refused because the draft changed + * since they loaded it, and offers the two ruled ways out: reload the saved + * version, or overwrite it with this buffer after a second, explicit + * confirmation. Closing it keeps the author's edits on screen, unsaved. + */ +export function DraftConflictDialog({ conflict, onChoose }: DraftConflictDialogProps): React.ReactElement { + const locale = useMetadataLocale(); + const [confirming, setConfirming] = React.useState(false); + const open = conflict !== null; + // Every way out lands the next conflict on the first step again. + const finish = (choice: DraftConflictChoice): void => { + setConfirming(false); + onChoose(choice); + }; + const vars = { + type: conflict ? translateMetadataType(conflict.type, locale) : '', + name: conflict?.name ?? '', + }; + return ( + { + if (!next) finish('cancel'); + }} + > + + +
+ +
+ + {confirming + ? t('engine.draftConflict.overwriteTitle', locale) + : t('engine.draftConflict.title', locale)} + + + {confirming + ? tFormat('engine.draftConflict.overwriteDescription', locale, vars) + : tFormat('engine.draftConflict.description', locale, vars)} + +
+
+ + + {confirming ? ( + <> + + + + ) : ( + <> + + + + + )} + + + + ); +} diff --git a/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx index 8ca508067f..5d701f4452 100644 --- a/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx +++ b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx @@ -76,6 +76,8 @@ import { CapabilityMultiSelectField, parseCapabilityNames } from '@object-ui/fie import { PageShell } from './PageShell.js'; import { HistoryPanel } from './ResourceHistoryPage.js'; import { useMetadataClient, useMetadataTypes, type RichMetadataTypeEntry } from './useMetadata.js'; +// objectui#11773 — the package door's draft save sends the version it was built on. +import { useDraftSaveGuard } from './DraftConflictDialog.js'; import { t as translate, tFormat, useMetadataLocale } from './i18n.js'; import { PermissionAdvancedFacets } from './PermissionAdvancedFacets.js'; import { errorCodeIs } from '@object-ui/types'; @@ -436,6 +438,15 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, // is enough: the anchor is a JSON snapshot, so a copy is anchor-identical. setDraft({ ...next }); }, []); + // objectui#11773 — the version the package door's draft was saved at, sent as + // `If-Match` by its next draft save. A conflict's "reload" re-runs the load. + const [reloadNonce, setReloadNonce] = React.useState(0); + const reloadSet = React.useCallback(() => setReloadNonce((n) => n + 1), []); + const { + save: saveVersioned, + forget: forgetSetVersion, + dialog: draftConflictDialog, + } = useDraftSaveGuard(client, reloadSet); const [objects, setObjects] = React.useState([]); const [fieldsByObject, setFieldsByObject] = React.useState>({}); const [expanded, setExpanded] = React.useState>(new Set()); @@ -571,6 +582,8 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, } else { resetDraftBaseline(full); } + // objectui#11773 — a read serves no version: the next save is unpinned. + forgetSetVersion(); } catch (err: any) { setError(err?.message ?? String(err)); } finally { @@ -580,7 +593,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, return () => { cancelled = true; }; - }, [client, type, name, packageId, publishNonce, resetDraftBaseline]); + }, [client, type, name, packageId, publishNonce, resetDraftBaseline, reloadNonce, forgetSetVersion]); /* ── Lazy-load fields when an object is expanded ─────────── */ async function ensureFields(objectName: string) { @@ -855,10 +868,14 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, // (stamped with `packageId`) that the package's atomic Publish promotes, // exactly like the Data/Interfaces pillars — NOT a live record write. // Environment door (no packageId) stays live (config). - await client.save(type, payload.name, toSave, { + // objectui#11773 — the guard pins the package door's DRAFT save; the + // environment door's live write passes through it unpinned, as before. + const outcome = await saveVersioned(type, payload.name, toSave, { force, ...(packageId ? { mode: 'draft' as const, packageId } : {}), }); + // The author chose the saved version; the load replaces the matrix. + if (outcome === 'reloaded') return; if (packageId) { // The draft is now the pending truth for display; the published baseline // hasn't moved. Show what we just staged and let the surface count it. @@ -1439,6 +1456,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, )} + {draftConflictDialog} ); } diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.draftVersionConflict-11773.test.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.draftVersionConflict-11773.test.tsx new file mode 100644 index 0000000000..c5ef312983 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.draftVersionConflict-11773.test.tsx @@ -0,0 +1,215 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — the metadata designer's draft save sends the version its + * buffer was saved at, and its two 409s stay apart. + * + * The `/meta` draft door answers two different conflicts with the same status + * and they need different answers from the author: + * + * - `409 DESTRUCTIVE_CHANGE` — the body would drop or narrow data; the editor's + * own confirmation re-sends it with `force`. Judged BEFORE the version. + * - `409 METADATA_CONFLICT` — the draft was saved elsewhere since this buffer + * was; the shared conflict dialog offers reload or overwrite. + * + * The door double answers the way the 17.7.0 door was measured to (readings on + * the pull request), and its refusals are parsed by the REAL `MetadataClient`. + * Only the page canvas is stubbed, to hand the test a way to dirty the draft; + * the autosave, the save door and both dialogs are shipping code. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { MetadataClient, type MetadataError } from '@object-ui/data-objectstack'; + +const PAGE = { + name: 'home', + label: 'Home', + type: 'home', + template: 'default', + regions: [{ name: 'main', components: [{ type: 'text', id: 'b1' }] }], +}; + +/** A label the door double treats as dropping data, so the destructive path is reachable. */ +const DESTRUCTIVE_LABEL = 'Drops a column'; + +interface SaveCall { + body: Record; + force: boolean; + ifMatch: string | undefined; +} + +const door = { + draft: null as null | { body: Record; version: string }, + seq: 0, + saves: [] as SaveCall[], +}; + +async function parsedRefusal(wire: unknown): Promise { + const client = new MetadataClient({ + baseUrl: 'http://localhost:3000', + fetch: (async () => + new Response(JSON.stringify(wire), { status: 409, headers: { 'content-type': 'application/json' } })) as unknown as typeof fetch, + }); + return client.save('page', 'home', {}, { mode: 'draft' }).then( + () => { + throw new Error('the stub door accepted the save'); + }, + (e: unknown) => e as MetadataError, + ); +} + +function writeDraft(body: Record): string { + door.seq += 1; + const version = `hmac-sha256:v${door.seq}`; + door.draft = { body, version }; + return version; +} + +const mockClient = { + list: vi.fn(async () => []), + listDrafts: vi.fn(async () => []), + layered: vi.fn(async () => ({ effective: PAGE, code: PAGE, editable: true })), + // The draft read serves the body and no version (measured on 17.7.0). + getDraft: vi.fn(async () => (door.draft ? { type: 'page', name: 'home', item: JSON.parse(JSON.stringify(door.draft.body)) } : null)), + get: vi.fn(async () => null), + references: vi.fn(async () => []), + publish: vi.fn(async () => ({ success: true })), + reset: vi.fn(async () => ({})), + save: vi.fn(async (_type: string, _name: string, item: unknown, options?: { force?: boolean; ifMatch?: string }) => { + const body = JSON.parse(JSON.stringify(item)) as Record; + const ifMatch = options?.ifMatch; + const force = !!options?.force; + door.saves.push({ body, force, ifMatch }); + if (body.label === DESTRUCTIVE_LABEL && !force) { + throw await parsedRefusal({ + error: "page/home would drop or transform existing data: Field 'subject' removed. — re-submit with ?force=true to proceed.", + code: 'DESTRUCTIVE_CHANGE', + issues: [{ code: 'field_removed', field: 'subject', message: "Field 'subject' removed." }], + }); + } + const head = door.draft?.version ?? null; + if (ifMatch !== undefined && ifMatch !== head) { + throw await parsedRefusal({ + error: `page/home has been modified since you loaded it. The version token sent is not the current version (current is ${head}).`, + code: 'METADATA_CONFLICT', + }); + } + const version = writeDraft(body); + return { success: true, version, seq: door.seq, state: 'draft', message: `Saved page 'home' [seq=${door.seq}]` }; + }), +}; + +vi.mock('./useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ + entries: [{ type: 'page', name: 'page', label: 'Page', allowOrgOverride: true, schema: { required: [] } }], + }), + }; +}); + +import { MetadataResourceEditPage } from './ResourceEditPage'; +import { registerMetadataPreview, getMetadataPreview } from './preview-registry'; + +/** Canvas stand-in: its only job is to dirty the draft, which arms the real autosave. */ +function StubPageCanvas({ onPatch }: { onPatch?: (patch: Record) => void }) { + return ( + <> + {['One', 'Two', 'Three', DESTRUCTIVE_LABEL].map((label) => ( + + ))} + + ); +} + +const realPagePreview = getMetadataPreview('page'); + +beforeEach(() => { + door.draft = null; + door.seq = 0; + door.saves.length = 0; + for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear(); + registerMetadataPreview('page', StubPageCanvas as never); + window.history.replaceState(null, '', '/metadata/page/home?package=com.acme.app'); +}); + +afterEach(() => { + cleanup(); + if (realPagePreview) registerMetadataPreview('page', realPagePreview); + window.history.replaceState(null, '', '/'); +}); + +function renderEditor() { + render( + + + , + ); +} + +/** Patch the label and wait for the autosave that carries it to be sent. */ +async function patchAndSave(label: string): Promise { + const before = door.saves.length; + fireEvent.click(await screen.findByRole('button', { name: `label ${label}` }, { timeout: 8000 })); + await waitFor(() => expect(door.saves.length).toBe(before + 1), { timeout: 8000 }); + return door.saves[door.saves.length - 1]!; +} + +describe('MetadataResourceEditPage — draft saves carry their version (objectui#11773)', () => { + it('the first autosave is unpinned; each later one sends the version the one before it received', async () => { + renderEditor(); + expect((await patchAndSave('One')).ifMatch).toBeUndefined(); + await waitFor(() => expect(door.draft?.version).toBe('hmac-sha256:v1'), { timeout: 8000 }); + expect((await patchAndSave('Two')).ifMatch).toBe('hmac-sha256:v1'); + await waitFor(() => expect(door.draft?.version).toBe('hmac-sha256:v2'), { timeout: 8000 }); + expect((await patchAndSave('Three')).ifMatch).toBe('hmac-sha256:v2'); + expect(screen.queryByTestId('draft-conflict-dialog')).toBeNull(); + }); + + it('a draft saved elsewhere refuses the stale save with the CONFLICT dialog, not the destructive one', async () => { + renderEditor(); + await patchAndSave('One'); + await waitFor(() => expect(door.draft?.version).toBe('hmac-sha256:v1'), { timeout: 8000 }); + // Another editor's save of the same draft. + writeDraft({ ...PAGE, label: 'Theirs' }); + + const stale = await patchAndSave('Two'); + expect(stale.ifMatch).toBe('hmac-sha256:v1'); + expect(await screen.findByTestId('draft-conflict-dialog', undefined, { timeout: 8000 })).toHaveTextContent('home'); + expect(screen.queryByText('Destructive change detected')).toBeNull(); + expect(door.draft?.body.label).toBe('Theirs'); + + fireEvent.click(screen.getByTestId('draft-conflict-reload')); + await waitFor(() => expect(screen.queryByTestId('draft-conflict-dialog')).toBeNull(), { timeout: 8000 }); + // The reload re-read the draft (its version was not served), so the next + // save is unpinned and starts from their body. + await waitFor(() => expect(mockClient.getDraft.mock.calls.length).toBeGreaterThanOrEqual(3), { timeout: 8000 }); + expect((await patchAndSave('Three')).ifMatch).toBeUndefined(); + }); + + it('control: a destructive change still opens its own confirmation, and the forced retry keeps the version', async () => { + renderEditor(); + await patchAndSave('One'); + await waitFor(() => expect(door.draft?.version).toBe('hmac-sha256:v1'), { timeout: 8000 }); + + const refused = await patchAndSave(DESTRUCTIVE_LABEL); + expect([refused.force, refused.ifMatch]).toEqual([false, 'hmac-sha256:v1']); + expect(await screen.findByText('Destructive change detected', undefined, { timeout: 8000 })).toBeInTheDocument(); + expect(screen.queryByTestId('draft-conflict-dialog')).toBeNull(); + + const before = door.saves.length; + fireEvent.click(screen.getByRole('button', { name: 'Force save' })); + await waitFor(() => expect(door.saves.length).toBe(before + 1), { timeout: 8000 }); + const forced = door.saves[door.saves.length - 1]!; + expect([forced.force, forced.ifMatch, forced.body.label]).toEqual([true, 'hmac-sha256:v1', DESTRUCTIVE_LABEL]); + await waitFor(() => expect(door.draft?.body.label).toBe(DESTRUCTIVE_LABEL), { timeout: 8000 }); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index 145124f8fe..a518e1a3c1 100644 --- a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -109,6 +109,8 @@ import { useMetadataTypes, type RichMetadataTypeEntry, } from './useMetadata.js'; +// objectui#11773 — the draft save's optimistic-concurrency guard and its dialog. +import { useDraftSaveGuard } from './DraftConflictDialog.js'; import { getMetadataResource, resolveResourceConfig, @@ -675,6 +677,15 @@ function MetadataResourceEditPageImpl({ // Bumped by destructive operations (rollback / discard-draft) to // force the load effect to refetch layered + draft state. const [reloadKey, setReloadKey] = React.useState(0); + // objectui#11773 — the version the editor's draft was saved at, sent as + // `If-Match` by every draft save of an existing item. A conflict's "reload" + // re-runs the load effect, the same way a discard does. + const reloadFromServer = React.useCallback(() => setReloadKey((k) => k + 1), []); + const { + save: saveDraftVersioned, + forget: forgetDraftVersion, + dialog: draftConflictDialog, + } = useDraftSaveGuard(client, reloadFromServer); // Form edit mode. The form is read-only by default — admins land in a // "view" state and must click Edit to mutate, mirroring the Salesforce / @@ -1054,6 +1065,8 @@ function MetadataResourceEditPageImpl({ const initial = config.toDraft ? config.toDraft(rawInitial) : rawInitial; setDraft(initial); draftSnapshotRef.current = initial; + // objectui#11773 — a read serves no version: the next save is unpinned. + forgetDraftVersion(); setHasDraft(!!draftReal); setLoading(false); } catch (err: any) { @@ -1077,7 +1090,7 @@ function MetadataResourceEditPageImpl({ return () => { cancelled = true; }; - }, [client, type, name, ownerPackageId, createMode, reloadKey, locale]); + }, [client, type, name, ownerPackageId, createMode, reloadKey, locale, forgetDraftVersion]); // Lazy-load references the first time the References sheet opens. // @@ -1491,11 +1504,20 @@ function MetadataResourceEditPageImpl({ // stamps it on create and preserves an existing binding on update, so // env-local overlays (no `?package=`) are unaffected. const activePackage = readActivePackageBinding(); - await client.save(type, savedName, itemToSave, { + const saveOptions = { force, - mode: 'draft', + mode: 'draft' as const, ...(activePackage ? { packageId: activePackage } : {}), - }); + }; + // objectui#11773 — a create sends no `If-Match` (the door cannot pin "no + // row yet"); a save of the item this editor loaded sends the version its + // last save received. + if (createMode) { + await client.save(type, savedName, itemToSave, saveOptions); + } else if ((await saveDraftVersioned(type, savedName, itemToSave, saveOptions)) === 'reloaded') { + // The author chose the saved version; the load effect replaces the draft. + return; + } // Refresh layered + draft state after save — scope to the same package // as the initial load (ADR-0048) so a same-name collision re-reads this // package's own row, not another's. @@ -1646,6 +1668,8 @@ function MetadataResourceEditPageImpl({ setError(null); try { await client.reset(type, name); + // objectui#11773 — the buffer below is re-read, so no version describes it. + forgetDraftVersion(); if (isResetSemantic) { const lay = await client.layered(type, name); setLayered(lay); @@ -1704,6 +1728,8 @@ function MetadataResourceEditPageImpl({ const fresh = config.toDraft ? config.toDraft(rawFresh) : rawFresh; setDraft(fresh); draftSnapshotRef.current = fresh; + // objectui#11773 — the publish dropped the draft the version named. + forgetDraftVersion(); } catch (err: any) { setError(err?.message ?? String(err)); } finally { @@ -1721,6 +1747,8 @@ function MetadataResourceEditPageImpl({ setError(null); try { await client.reset(type, name, { state: 'draft' }); + // objectui#11773 — the discard dropped the draft the version named. + forgetDraftVersion(); const lay = await client.layered(type, name); setLayered(lay); const fresh = (lay.effective ?? lay.code ?? {}) as Record; @@ -3103,6 +3131,9 @@ function MetadataResourceEditPageImpl({ + {/* objectui#11773 — the draft-version conflict, apart from the + destructive-change confirmation above: two different 409s. */} + {draftConflictDialog} ); } diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index f82bfd79ab..cf231ac702 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -2678,6 +2678,21 @@ const ENGINE_STRINGS_EN: Record = { 'engine.studio.saveDraft': 'Save draft', 'engine.studio.more': 'More', 'engine.studio.autoSaving': 'Saving…', + // objectui#11773 — the draft-save version conflict (`DraftConflictDialog.tsx`): + // a draft save the server refused because the draft was saved elsewhere after + // this editor opened it. + 'engine.draftConflict.title': 'This draft changed since you opened it', + 'engine.draftConflict.description': + 'The draft of {type} “{name}” was saved elsewhere — by someone else or in another tab — after you opened it. Reload the saved version (your unsaved edits here are dropped), or overwrite it with yours.', + 'engine.draftConflict.reload': 'Reload saved version', + 'engine.draftConflict.overwrite': 'Overwrite…', + 'engine.draftConflict.keepEditing': 'Keep editing', + 'engine.draftConflict.overwriteTitle': 'Overwrite the saved draft?', + 'engine.draftConflict.overwriteDescription': + 'Your version of {type} “{name}” will replace the saved draft, and the changes saved after you opened it will be lost.', + 'engine.draftConflict.overwriteConfirm': 'Overwrite', + 'engine.draftConflict.back': 'Back', + 'engine.draftConflict.notSaved': 'Not saved: {type} “{name}” was saved elsewhere after you opened it.', 'engine.studio.data.tab.advanced': 'Advanced', // Standard create-dialog field labels (shared by object / app / flow / permission). 'engine.studio.app.nameLabel': 'App name', @@ -5591,6 +5606,18 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.studio.saveDraft': '保存草稿', 'engine.studio.more': '更多', 'engine.studio.autoSaving': '保存中…', + 'engine.draftConflict.title': '此草稿在你打开后已被修改', + 'engine.draftConflict.description': + '{type}「{name}」的草稿在你打开后已在别处保存（其他人或另一个标签页）。可以重新加载已保存的版本（此处未保存的修改将丢弃），或用你的版本覆盖它。', + 'engine.draftConflict.reload': '重新加载已保存版本', + 'engine.draftConflict.overwrite': '覆盖…', + 'engine.draftConflict.keepEditing': '继续编辑', + 'engine.draftConflict.overwriteTitle': '覆盖已保存的草稿？', + 'engine.draftConflict.overwriteDescription': + '你的{type}「{name}」将替换已保存的草稿，你打开之后保存的那些修改会丢失。', + 'engine.draftConflict.overwriteConfirm': '覆盖', + 'engine.draftConflict.back': '返回', + 'engine.draftConflict.notSaved': '未保存：{type}「{name}」在你打开后已在别处保存。', 'engine.studio.data.tab.advanced': '高级', // Standard create-dialog field labels (shared by object / app / flow / permission). 'engine.studio.app.nameLabel': '应用名称', diff --git a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx index 0491eb3451..59bf3ea144 100644 --- a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx @@ -28,6 +28,8 @@ import { toast } from 'sonner'; import { SchemaForm } from '../metadata-admin/SchemaForm.js'; import { getMetadataDefaultInspector } from '../metadata-admin/default-inspector-registry.js'; import { useMetadataClient } from '../metadata-admin/useMetadata.js'; +// objectui#11773 — the hook's draft save sends the version it was built on. +import { useDraftSaveGuard } from '../metadata-admin/DraftConflictDialog.js'; import { t, tFormat, useMetadataLocale } from '../metadata-admin/i18n.js'; // `formatMetadataError` is the one metadata-save error reader (objectui#11302). import { extractDraftBody, formatMetadataError } from '@object-ui/data-objectstack'; @@ -75,10 +77,16 @@ export function ObjectHooksPanel({ packageId, disabled, hookSchema, + publishNonce = 0, }: { objectName: string; packageId: string; disabled?: boolean; + /** + * objectui#11773 — bumped by a package publish, which drops every draft and + * with them the version this panel's last save received. + */ + publishNonce?: number; /** * The live server JSONSchema for the `hook` type (`/meta/types`). Drives the * SchemaForm so the fields, enums and grouping come from the real hook @@ -99,6 +107,19 @@ export function ObjectHooksPanel({ const [dirty, setDirty] = React.useState(false); const [saving, setSaving] = React.useState(false); const [nonce, setNonce] = React.useState(0); + // objectui#11773 — the version the open hook's draft was saved at, sent as + // `If-Match` by its next save. The list re-read that follows this panel's own + // save reads back what it wrote, so the version survives it; a conflict's + // "reload" re-reads the list, and a publish forgets the version. + const reloadHooks = React.useCallback(() => setNonce((n) => n + 1), []); + const { + save: saveHookDraft, + forget: forgetHookVersion, + dialog: hookConflictDialog, + } = useDraftSaveGuard(client, reloadHooks); + React.useEffect(() => { + forgetHookVersion(); + }, [publishNonce, forgetHookVersion]); /* ─── Blocking CEL verdicts → this panel's OWN Save (objectui#4527) ──────── * @@ -176,7 +197,7 @@ export function ObjectHooksPanel({ setSaving(true); setError(null); try { - await client.save('hook', String(draft.name), draft, { mode: 'draft', packageId }); + if ((await saveHookDraft('hook', String(draft.name), draft, { mode: 'draft', packageId })) === 'reloaded') return; toast.success(tFormat('engine.studio.hooks.saved', locale, { label: String(draft.label || draft.name) })); setDirty(false); setNonce((n) => n + 1); @@ -185,7 +206,7 @@ export function ObjectHooksPanel({ } finally { setSaving(false); } - }, [client, draft, packageId, locale]); + }, [saveHookDraft, draft, packageId, locale]); const addHook = React.useCallback(async () => { const name = nextHookName(objectName, hooks.map((h) => String(h.name ?? ''))); @@ -215,6 +236,7 @@ export function ObjectHooksPanel({ return (
+ {hookConflictDialog} {/* hook list */}
diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.draftVersionConflict-11773.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.draftVersionConflict-11773.test.tsx new file mode 100644 index 0000000000..1a13b1fd60 --- /dev/null +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.draftVersionConflict-11773.test.tsx @@ -0,0 +1,306 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — two editors of one object in Studio's Data pillar no longer + * overwrite each other's draft in silence. + * + * The defect, as filed: tab A saved a field, tab B (opened earlier) saved its + * stale copy, and the server draft lost A's field with no 409, banner or toast, + * because no draft save sent the version it was built on. + * + * The server double answers the way the `/meta` draft door of + * `@objectstack/*` 17.7.0 was measured to (readings on the pull request): a + * draft save returns its `version`; an `If-Match` that is not the current + * draft's version is `409 METADATA_CONFLICT`; a save with no `If-Match` is + * last-writer-wins; the draft READ serves no version. Its refusal is parsed by + * the REAL `MetadataClient`, so the error the pillar catches is production's. + * + * Every editor below has saved at least once before the race. The FIRST save + * after a load has no version to send — the draft read serves none on 17.7.0 — + * which is the half of the card this suite cannot pin and the pull request + * names as the server's. + * + * The two editors are two mounted `DataPillar`s over one server double: each + * pillar holds its own buffer and its own version, exactly as two tabs do. + * The records grid is a double that hands the pillar a column order through + * the grid's own authoring context (the drag mechanics are the data table's). + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor, within, act } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { MetadataClient, type MetadataError } from '@object-ui/data-objectstack'; + +const PKG = 'com.acme.app'; + +const OBJECT = { + name: 'acme_task', + label: 'Task', + fields: [ + { name: 'title', label: 'Title', type: 'text' }, + { name: 'status', label: 'Status', type: 'text' }, + ], +}; + +interface SaveCall { + type: string; + name: string; + body: Record; + ifMatch: string | undefined; +} + +const server = vi.hoisted(() => ({ + active: new Map>(), + drafts: new Map; version: string }>(), + saves: [] as SaveCall[], + seq: 0, +})); + +/** + * The 409 the door answers, run through the real client's error parser. The + * prose is the door's own; nothing in the guard reads it. + */ +async function conflictRefusal(type: string, name: string, head: string | null): Promise { + const client = new MetadataClient({ + baseUrl: 'http://localhost:3000', + fetch: (async () => + new Response( + JSON.stringify({ + error: `${type}/${name} has been modified since you loaded it. The version token sent is not the current version (current is ${head}).`, + code: 'METADATA_CONFLICT', + }), + { status: 409, headers: { 'content-type': 'application/json' } }, + )) as unknown as typeof fetch, + }); + return client.save(type, name, {}, { mode: 'draft' }).then( + () => { + throw new Error('the stub door accepted the save'); + }, + (e: unknown) => e as MetadataError, + ); +} + +const mockClient = vi.hoisted(() => { + const k = (type: string, name: string) => `${type}/${name}`; + return { + list: vi.fn(async (type: string) => + [...server.active.entries()] + .filter(([key]) => key.startsWith(`${type}/`)) + .map(([, row]) => ({ name: row.name, label: row.label ?? row.name })), + ), + listDrafts: vi.fn(async () => []), + listTypes: vi.fn(async () => ({ entries: [] })), + get: vi.fn(async () => null), + references: vi.fn(async () => []), + layered: vi.fn(async (type: string, name: string) => { + const eff = server.active.get(k(type, name)) ?? null; + return { code: null, overlay: eff, overlayScope: eff ? 'env' : null, effective: eff, editable: true, deletable: true, resettable: false, lock: 'none' }; + }), + // The draft read serves the body and NO version (measured on 17.7.0). + getDraft: vi.fn(async (type: string, name: string) => { + const draft = server.drafts.get(k(type, name)); + if (draft) return { type, name, item: JSON.parse(JSON.stringify(draft.body)) as Record }; + throw Object.assign(new Error(`No pending draft exists for ${type}/${name}.`), { code: 'NO_DRAFT', status: 404 }); + }), + save: vi.fn(async (type: string, name: string, item: unknown, options?: { ifMatch?: string }) => { + const body = JSON.parse(JSON.stringify(item)) as Record; + const ifMatch = options?.ifMatch; + server.saves.push({ type, name, body, ifMatch }); + const head = server.drafts.get(k(type, name))?.version ?? null; + if (ifMatch !== undefined && ifMatch !== head) throw await conflictRefusal(type, name, head); + server.seq += 1; + const version = `hmac-sha256:v${server.seq}`; + server.drafts.set(k(type, name), { body, version }); + return { success: true, version, seq: server.seq, state: 'draft', message: `Saved ${type} '${name}' [seq=${server.seq}]` }; + }), + publish: vi.fn(async () => ({ success: true })), + reset: vi.fn(async () => ({})), + }; +}); + +vi.mock('../metadata-admin/useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useMetadataClient: () => mockClient, useMetadataTypes: () => ({ entries: [] }) }; +}); + +vi.mock('./packages-io', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, fetchPackages: vi.fn(async () => []) }; +}); + +vi.mock('@object-ui/react', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useAdapter: () => dataSource }; +}); + +// The records grid: a double that reverses the columns it was handed through +// the grid's authoring context, the way a header drag reorders them. +vi.mock('@object-ui/plugin-view', async (importOriginal) => { + const mod = await importOriginal>(); + const { useGridFieldAuthoring } = await import('@object-ui/components'); + function GridDouble({ schema }: { schema?: { table?: { fields?: string[] } } }) { + const authoring = useGridFieldAuthoring(); + const cols = schema?.table?.fields ?? []; + return ( + + ); + } + return { ...mod, ObjectView: GridDouble }; +}); + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), dismiss: vi.fn() } })); + +import { DataPillar } from './StudioDesignSurface'; +import { createEmptyDataSource } from './__tests__/emptyDataSource'; +import { readFields } from '../metadata-admin/previews/object-fields-io'; + +const dataSource = createEmptyDataSource(); + +beforeEach(() => { + server.active.clear(); + server.drafts.clear(); + server.saves.length = 0; + server.seq = 0; + for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear(); + server.active.set(`object/${OBJECT.name}`, JSON.parse(JSON.stringify(OBJECT))); +}); + +afterEach(cleanup); + +/** One editor: a mounted Data pillar, as one browser tab holds it. */ +function openEditor(): HTMLElement { + const { container } = render( + + + , + ); + return container; +} + +const fieldNames = (body: Record) => readFields(body.fields).entries.map((e) => e.name); +const fieldCount = (editor: HTMLElement) => within(editor).getByText(/^\d+ fields$/).textContent; +const serverDraftFields = () => fieldNames(server.drafts.get(`object/${OBJECT.name}`)!.body); + +async function loaded(editor: HTMLElement, count: string): Promise { + await waitFor(() => expect(fieldCount(editor)).toBe(count), { timeout: 8000 }); +} + +/** Add a field and wait until the autosave carrying it has landed. */ +async function addFieldAndSave(editor: HTMLElement): Promise { + const before = server.saves.length; + fireEvent.click(within(editor).getByTitle(/^Add a field/)); + await waitFor(() => expect(server.saves.length).toBe(before + 1), { timeout: 8000 }); + await waitFor(() => expect(within(editor).queryByTestId('data-autosaving')).toBeNull(), { timeout: 8000 }); +} + +/** Reverse the grid's columns (a save the pillar sends itself) and wait for it. */ +async function reorderAndSave(editor: HTMLElement): Promise { + const before = server.saves.length; + fireEvent.click(await within(editor).findByRole('button', { name: 'Reverse columns' }, { timeout: 8000 })); + await waitFor(() => expect(server.saves.length).toBe(before + 1), { timeout: 8000 }); + await waitFor(() => expect(within(editor).queryByTestId('data-autosaving')).toBeNull(), { timeout: 8000 }); +} + +/** + * The race, with both editors past their first save: A saves, B opens on A's + * draft and saves its own change, then A — still holding the copy it saved — + * edits again. + */ +async function raceTwoEditors(): Promise<{ a: HTMLElement; b: HTMLElement }> { + const a = openEditor(); + await loaded(a, '2 fields'); + await addFieldAndSave(a); // A: + field_3 + const b = openEditor(); + await loaded(b, '3 fields'); + await reorderAndSave(b); // B: status before title + expect(serverDraftFields()).toEqual(['status', 'title', 'field_3']); + fireEvent.click(within(a).getByTitle(/^Add a field/)); // A's stale copy + field_4 + await screen.findByTestId('draft-conflict-dialog', undefined, { timeout: 8000 }); + return { a, b }; +} + +describe('Data pillar — a draft saved elsewhere is not overwritten in silence (objectui#11773)', () => { + it('one editor: every autosave after the first sends the version the one before it received', async () => { + const a = openEditor(); + await loaded(a, '2 fields'); + await addFieldAndSave(a); + await addFieldAndSave(a); + await reorderAndSave(a); + expect(server.saves.map((s) => s.ifMatch)).toEqual([undefined, 'hmac-sha256:v1', 'hmac-sha256:v2']); + expect(screen.queryByTestId('draft-conflict-dialog')).toBeNull(); + }); + + it('two editors: the stale save is refused with the conflict dialog, and the other editor\'s change survives', async () => { + const { a } = await raceTwoEditors(); + const refused = server.saves[server.saves.length - 1]!; + expect(refused.ifMatch).toBe('hmac-sha256:v1'); + expect(fieldNames(refused.body)).toEqual(['title', 'status', 'field_3', 'field_4']); + // Nothing was written over B's draft. + expect(serverDraftFields()).toEqual(['status', 'title', 'field_3']); + expect(screen.getByTestId('draft-conflict-dialog')).toHaveTextContent('acme_task'); + + fireEvent.click(screen.getByTestId('draft-conflict-cancel')); + await waitFor(() => expect(screen.queryByTestId('draft-conflict-dialog')).toBeNull(), { timeout: 8000 }); + // Keep editing: A's edit stays on screen, unsaved, and the refusal says so. + expect(fieldCount(a)).toBe('4 fields'); + expect(within(a).getByText(/^Not saved:/)).toBeInTheDocument(); + expect(serverDraftFields()).toEqual(['status', 'title', 'field_3']); + }); + + it('"reload" replaces the stale buffer with the saved draft, and the next save is unpinned', async () => { + const { a } = await raceTwoEditors(); + const savesBefore = server.saves.length; + fireEvent.click(screen.getByTestId('draft-conflict-reload')); + await waitFor(() => expect(screen.queryByTestId('draft-conflict-dialog')).toBeNull(), { timeout: 8000 }); + await loaded(a, '3 fields'); + expect(server.saves.length).toBe(savesBefore); + + await addFieldAndSave(a); + const next = server.saves[server.saves.length - 1]!; + expect(next.ifMatch).toBeUndefined(); + // Built on B's draft: B's column order is kept. + expect(fieldNames(next.body)).toEqual(['status', 'title', 'field_3', 'field_4']); + }); + + it('"overwrite" asks again, then re-sends without If-Match and wins; the next save is pinned to it', async () => { + const { a } = await raceTwoEditors(); + fireEvent.click(screen.getByTestId('draft-conflict-overwrite')); + // The second, explicit confirmation — nothing is sent until it is given. + const savesBefore = server.saves.length; + expect(await screen.findByTestId('draft-conflict-overwrite-confirm')).toBeInTheDocument(); + expect(server.saves.length).toBe(savesBefore); + await act(async () => { + fireEvent.click(screen.getByTestId('draft-conflict-overwrite-confirm')); + }); + await waitFor(() => expect(server.saves.length).toBe(savesBefore + 1), { timeout: 8000 }); + const overwrite = server.saves[server.saves.length - 1]!; + expect(overwrite.ifMatch).toBeUndefined(); + expect(serverDraftFields()).toEqual(['title', 'status', 'field_3', 'field_4']); + await waitFor(() => expect(within(a).queryByTestId('data-autosaving')).toBeNull(), { timeout: 8000 }); + + await addFieldAndSave(a); + expect(server.saves[server.saves.length - 1]!.ifMatch).toBe(`hmac-sha256:v${server.seq - 1}`); + }); +}); + +describe('Data pillar — a create sends no If-Match (objectui#11773)', () => { + it('the new object\'s skeleton is sent unpinned; its first edit after the load is unpinned, the next is pinned', async () => { + server.active.clear(); + const a = openEditor(); + fireEvent.click(await within(a).findByTestId('empty-state-new-object', undefined, { timeout: 8000 })); + const dialog = await screen.findByRole('dialog'); + fireEvent.change(dialog.querySelectorAll('input')[0]!, { target: { value: 'Ticket' } }); + fireEvent.click(within(dialog).getByRole('button', { name: /save as draft/i })); + await waitFor(() => expect(server.saves).toHaveLength(1), { timeout: 8000 }); + expect(server.saves[0]!.ifMatch).toBeUndefined(); + + await loaded(a, '1 fields'); + await addFieldAndSave(a); + await addFieldAndSave(a); + expect(server.saves.map((s) => s.ifMatch)).toEqual([undefined, undefined, 'hmac-sha256:v2']); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index 09ff292a04..bc5ceeaf0b 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -99,6 +99,8 @@ import { import { getMetadataDefaultInspector } from '../metadata-admin/default-inspector-registry.js'; import { getMetadataResource } from '../metadata-admin/registry.js'; import { useMetadataClient, useMetadataTypes } from '../metadata-admin/useMetadata.js'; +// objectui#11773 — every draft save of an existing item sends the version its buffer was built on. +import { useDraftSaveGuard } from '../metadata-admin/DraftConflictDialog.js'; import { DESIGNER_SURFACE_PARAM, formatSurfaceParam, @@ -2081,6 +2083,34 @@ export function InterfacesPillar({ ); const [navHasDraft, setNavHasDraft] = React.useState(false); const [navSaving, setNavSaving] = React.useState(false); + // objectui#11773 — two buffers, two guards: the open leaf's `draft` and the + // app document `appDraft` the nav editor saves. Each sends the version its + // own buffer was saved at; a conflict's "reload" re-runs that buffer's load. + const [leafReloadNonce, setLeafReloadNonce] = React.useState(0); + const reloadLeafDraft = React.useCallback(() => setLeafReloadNonce((n) => n + 1), []); + const { + save: saveLeafDraft, + forget: forgetLeafVersion, + dialog: leafConflictDialog, + } = useDraftSaveGuard(client, reloadLeafDraft); + const [navReloadNonce, setNavReloadNonce] = React.useState(0); + // A reload replaces the buffer even over an unsent edit: the author chose + // the saved version over it. + const reloadNavDraft = React.useCallback(() => { + setNavDirty(false); + setNavReloadNonce((n) => n + 1); + }, []); + const { + save: saveNavDraft, + forget: forgetNavVersion, + dialog: navConflictDialog, + } = useDraftSaveGuard(client, reloadNavDraft); + // The app load also re-reads after every draft save in the package (the + // `draftNonce` it keys on), its own included. The re-read that follows this + // pillar's own nav save installs what that save wrote, so the version stays; + // any other install forgets it. Holds the `publishNonce` the save landed + // under: a publish in between dropped the draft, version and all. + const navEchoRef = React.useRef(null); // objectui#7255 — the copilot dock shares this document, so a turn that // staged/published metadata converges the rail here instead of waiting for a // page reload. HELD while the nav editor has unsaved (or in-flight) edits: @@ -2310,6 +2340,10 @@ export function InterfacesPillar({ if (!isSameApp || !navCommittedRef.current.dirty) { setAppDraft(body); setAppDraftFor(`app:${name}`); + // objectui#11773 — a read serves no version, unless it is the read-back + // of this pillar's own nav save (see `navEchoRef`). + if (!isSameApp || navEchoRef.current !== publishNonce) forgetNavVersion(); + navEchoRef.current = null; } navBaselineRef.current = body; setNavHasDraft(!!appDraftBody); @@ -2347,7 +2381,7 @@ export function InterfacesPillar({ return () => { cancelled = true; }; - }, [client, packageId, publishNonce, draftNonce, metadataRefreshNonce]); + }, [client, packageId, publishNonce, draftNonce, metadataRefreshNonce, navReloadNonce, forgetNavVersion]); const Preview = getMetadataPreview(current?.type ?? ''); // Studio-canvas surface override: the SAME type can render as a different @@ -2430,6 +2464,7 @@ export function InterfacesPillar({ // page's draft. setDraft({}); setDraftFor(leafKeyOf(current)); + forgetLeafVersion(); setHasDraft(false); setIfDirty(false); return; @@ -2461,6 +2496,8 @@ export function InterfacesPillar({ // spread over `effective` resurrects every key the draft deleted. setDraft(body ?? baseline); setDraftFor(leafKeyOf(current)); + // objectui#11773 — a read serves no version: the next save is unpinned. + forgetLeafVersion(); setHasDraft(!!body); setIfDirty(false); } catch (e) { @@ -2482,7 +2519,7 @@ export function InterfacesPillar({ // same rule. if (!settled) setLoading(false); }; - }, [client, current, isEditable, publishNonce]); + }, [client, current, isEditable, publishNonce, leafReloadNonce, forgetLeafVersion]); // objectui#5813 — a local dirty flag so auto-save only arms after a real // edit, never on the load-effect's own setDraft. @@ -2498,7 +2535,9 @@ export function InterfacesPillar({ if (!current) return; setSaving('draft'); try { - await client.save(current.type, current.name, interfacesSaveBody(current.type, draft), { mode: 'draft', packageId }); + const outcome = await saveLeafDraft(current.type, current.name, interfacesSaveBody(current.type, draft), { mode: 'draft', packageId }); + // objectui#11773 — the author chose the saved version; the load replaces the buffer. + if (outcome === 'reloaded') return; setHasDraft(true); // objectui#11204 — clean only if nothing was edited while it was in flight. if (sent.unmoved()) setIfDirty(false); @@ -2508,7 +2547,7 @@ export function InterfacesPillar({ } finally { setSaving(false); } - }, [client, current, draft, onDraftSaved]); + }, [saveLeafDraft, current, draft, onDraftSaved, packageId]); const { loaded: draftLoaded } = useDraftAutoSave({ // objectui#11232 — the leaf `doSave` addresses, `type:name`. target: leafKey, @@ -2541,7 +2580,10 @@ export function InterfacesPillar({ return typeof item.id === 'string' && item.id ? item : { ...item, id: `nav_item_${i + 1}` }; }); const saved = { ...appDraft, navigation: cleanedNav }; - await client.save('app', appName, saved, { mode: 'draft', packageId }); + const outcome = await saveNavDraft('app', appName, saved, { mode: 'draft', packageId }); + // objectui#11773 — the author chose the saved version; the load replaces the buffer. + if (outcome === 'reloaded') return; + navEchoRef.current = publishNonce; navBaselineRef.current = saved; setNavHasDraft(true); // objectui#11189, objectui#11204 — clean only if the buffer is still what @@ -2555,7 +2597,7 @@ export function InterfacesPillar({ } finally { setNavSaving(false); } - }, [client, appName, appDraft, onDraftSaved]); + }, [saveNavDraft, appName, appDraft, onDraftSaved, packageId, publishNonce]); // objectui#5813 — nav edits auto-save while edit mode is open. const { flush: flushNavSave } = useDraftAutoSave({ // objectui#11232 — the app `doNavSave` addresses. The package is this @@ -2973,6 +3015,8 @@ export function InterfacesPillar({ return (
+ {leafConflictDialog} + {navConflictDialog}