From 09b51fda356eea0b6d4c6f774bc86ac49be6fe03 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:17:04 +0000 Subject: [PATCH 1/5] fix(app-shell): Studio's first draft save after a load is pinned to the read, and a create sends If-None-Match (objectui#11773) Each Studio editor's load reads the draft at the address its saves write (`?package=`) and records what that read served: the draft's `version` (If-Match), or "no draft row" (If-None-Match: *). A create sends If-None-Match: *. The conflict dialog's Overwrite re-sends pinned to the 409's `currentVersion`, parsed by the spec's MetadataConflictErrorSchema. `@object-ui/data-objectstack`: MetadataClientSaveOptions gains `ifNoneMatch?: '*'`, never sent beside `ifMatch`; the `ifMatch` docblock names the token's real sources. Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- .changeset/11773-first-save-pinned.md | 19 + .../DraftConflictDialog.test.tsx | 328 ++++++++++++++---- .../metadata-admin/DraftConflictDialog.tsx | 238 +++++++++---- .../metadata-admin/PermissionMatrixEditor.tsx | 29 +- ...ditPage.createFirstWritePin-11773.test.tsx | 180 ++++++++++ ...itPage.draftVersionConflict-11773.test.tsx | 81 ++++- .../views/metadata-admin/ResourceEditPage.tsx | 56 ++- ...tHooksPanel.firstSavePinned-11773.test.tsx | 158 +++++++++ .../views/studio-design/ObjectHooksPanel.tsx | 59 +++- .../studio-design/ObjectListViewInspector.tsx | 15 +- ...urface.draftVersionConflict-11773.test.tsx | 95 +++-- .../studio-design/StudioDesignSurface.tsx | 93 +++-- ...etadata-client.firstWritePin-11773.test.ts | 99 ++++++ .../data-objectstack/src/metadata-client.ts | 46 ++- 14 files changed, 1230 insertions(+), 266 deletions(-) create mode 100644 .changeset/11773-first-save-pinned.md create mode 100644 packages/app-shell/src/views/metadata-admin/ResourceEditPage.createFirstWritePin-11773.test.tsx create mode 100644 packages/app-shell/src/views/studio-design/ObjectHooksPanel.firstSavePinned-11773.test.tsx create mode 100644 packages/data-objectstack/src/metadata-client.firstWritePin-11773.test.ts diff --git a/.changeset/11773-first-save-pinned.md b/.changeset/11773-first-save-pinned.md new file mode 100644 index 0000000000..9ab8bad289 --- /dev/null +++ b/.changeset/11773-first-save-pinned.md @@ -0,0 +1,19 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/data-objectstack': minor +--- + +Studio's first draft save after opening an item no longer replaces a draft that was saved elsewhere in the meantime (objectui#11773). + +Until now an editor's draft saves were pinned only from its second save on: the `/meta` draft read served no version, so two editors who each opened an item and saved it once overwrote each other's draft in silence. The `@objectstack/*` 18 draft read serves the version, and each editor's load now pins its first save to what it read: + +- an item with a pending draft is saved with `If-Match` and that draft's version; +- an item with no pending draft is saved with `If-None-Match: *`, so a draft another editor created in the meantime is refused, not overwritten. + +The editors read the draft at the package their saves write to (`?package=`), since a read's version describes the row a save to the same address compares against. This covers the metadata editor, the permission matrix's package door, the Studio pillars (objects, flows, interfaces and app navigation), the list-view panel and the hooks panel. + +A create sends `If-None-Match: *`, so creating an item under a name that already holds a draft is refused instead of writing the new skeleton over it. + +In the conflict dialog, **Overwrite** now re-sends the draft pinned to the version the refusal names (`currentVersion`), so a save that lands while the dialog is open is refused and asked about again rather than overwritten. A server whose refusal names no `currentVersion` is overwritten unpinned, as before. + +`@object-ui/data-objectstack`: `MetadataClientSaveOptions` gains `ifNoneMatch?: '*'`, sent as `If-None-Match: *`. It never travels with `ifMatch`: `MetadataClient.save` throws before sending a request that sets both. The `ifMatch` docblock now names the token's real sources (the item read's `version`, a save receipt's `version`, a conflict's `currentVersion`) instead of "the `checksum` returned by the last read". diff --git a/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.test.tsx b/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.test.tsx index e3af8b0171..a0e6bd4eaf 100644 --- a/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.test.tsx @@ -2,18 +2,24 @@ /** * 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): + * answers the way the `/meta` door of the published `@objectstack/*` + * 18.0.0-next.0 server 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; + * - the draft read (`GET …?state=draft&package=P`) serves `version`, the token + * of the draft at THAT address (`null` when that address holds none — a + * read naming no package of a package-bound draft serves `null`), and + * answers `404 NO_DRAFT` with no draft at all; + * - an `If-Match` that is not the current draft's token, or `If-None-Match: *` + * while a draft exists, is `409 { error, code: 'METADATA_CONFLICT', + * currentVersion }` (`currentVersion: null` with no draft row); the two + * headers together are `400`; * - 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 + * The client is the REAL `MetadataClient`, so the headers on the wire and the * parsed refusal are production's, not a hand-built error object. */ @@ -21,7 +27,9 @@ import { describe, it, expect, vi } from 'vitest'; import { MetadataClient } from '@object-ui/data-objectstack'; import { DraftVersionGuard, + draftPinOf, isDraftVersionConflict, + readDraftForSave, type DraftConflictChoice, } from './DraftConflictDialog'; @@ -31,6 +39,7 @@ interface SentPut { draft: boolean; force: boolean; ifMatch: string | null; + ifNoneMatch: string | null; body: Record; } @@ -38,28 +47,60 @@ 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 v = (seq: number) => `hmac-sha256:${seq.toString(16).padStart(64, '0')}`; + +/** The measured draft door, over `fetch`. */ +function measuredDoor(options: { conflictCarriesCurrentVersion?: boolean } = {}) { + const carriesCurrent = options.conflictCarriesCurrentVersion ?? true; const rows = new Map; version: string }>(); const puts: SentPut[] = []; + const reads: string[] = []; 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')}`; + const version = v(seq); rows.set(key, { body, version }); return version; }; + const conflict = (type: string, name: string, head: string | null) => + json(409, { + error: `${type}/${name} has been modified since you loaded it. Expected parent null but current is ${head}.`, + code: 'METADATA_CONFLICT', + ...(carriesCurrent ? { currentVersion: head } : {}), + }); const fetchImpl = (async (input: RequestInfo | URL, init?: RequestInit) => { const url = new URL(String(input)); const [, , , , type = '', name = ''] = url.pathname.split('/').map(decodeURIComponent); + const pkg = url.searchParams.get('package'); + if ((init?.method ?? 'GET') === 'GET') { + reads.push(`${url.pathname}?${url.searchParams.toString()}`); + if (type === '_drafts') { + const drafts = [...rows.keys()].filter((k) => k.startsWith('draft:')).map((k) => { + const [t, rest] = k.slice('draft:'.length).split('/'); + return { type: t, name: rest!.split('@')[0] }; + }); + return json(200, { drafts }); + } + if (url.searchParams.get('state') !== 'draft') return json(404, { error: 'not modelled' }); + const own = rows.get(rowKey(type, name, pkg, true)); + // Content falls back to any package's draft; the version is the address's. + const served = own ?? [...rows.entries()].find(([k]) => k.startsWith(`draft:${type}/${name}@`))?.[1]; + if (!served) return json(404, { error: `No pending draft exists for ${type}/${name}.`, code: 'NO_DRAFT' }); + return json(200, { type, name, item: served.body, version: own ? own.version : null }); + } 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 headers = new Headers(init.headers); + const ifMatch = headers.get('If-Match'); + const ifNoneMatch = headers.get('If-None-Match'); const body = JSON.parse(String(init.body)) as Record; - puts.push({ type, name, draft, force, ifMatch, body }); + puts.push({ type, name, draft, force, ifMatch, ifNoneMatch, body }); + if (ifMatch !== null && ifNoneMatch !== null) { + return json(400, { error: { code: 'VALIDATION_ERROR', message: 'Send If-Match or If-None-Match, not both.' } }); + } 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.`, @@ -67,14 +108,10 @@ function measuredDoor() { issues: [{ code: 'field_removed', field: 'subject', message: "Field 'subject' removed." }], }); } - const key = rowKey(type, name, url.searchParams.get('package'), draft); + const key = rowKey(type, name, pkg, 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', - }); - } + if (ifMatch !== null && (head === null || ifMatch.replace(/^"|"$/g, '') !== head)) return conflict(type, name, head); + if (ifNoneMatch !== null && head !== null) return conflict(type, name, head); const version = write(key, body); return json(200, { success: true, @@ -87,14 +124,15 @@ function measuredDoor() { return { client: new MetadataClient({ baseUrl: 'http://localhost:3000', fetch: fetchImpl }), puts, + reads, /** 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); + return 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). */ + /** A publish promotes the draft row and drops it (`currentVersion: null` after). */ publishDraft(type: string, name: string, pkg: string) { const key = rowKey(type, name, pkg, true); const row = rows.get(key); @@ -104,8 +142,9 @@ function measuredDoor() { }; } -function guardOver(door: ReturnType, answer: DraftConflictChoice = 'cancel') { - const ask = vi.fn(async () => answer); +function guardOver(door: ReturnType, ...answers: DraftConflictChoice[]) { + const queue = answers.length ? [...answers] : ['cancel' as const]; + const ask = vi.fn(async () => (queue.length > 1 ? queue.shift()! : queue[0]!)); const reload = vi.fn(); const guard = new DraftVersionGuard({ client: () => door.client, @@ -116,26 +155,134 @@ function guardOver(door: ReturnType, answer: DraftConflictC return { guard, ask, reload }; } -const DRAFT = { mode: 'draft' as const, packageId: 'com.acme.app' }; +const PKG = 'com.acme.app'; +const DRAFT = { mode: 'draft' as const, packageId: PKG }; + +/** One editor's load: the draft read at the save's address, recorded where the buffer is installed. */ +async function load(door: ReturnType, guard: DraftVersionGuard, name = 'acme_task') { + const read = await readDraftForSave(door.client, 'object', name, PKG); + guard.recordRead('object', name, PKG, read.pin); + return read; +} + +describe('DraftVersionGuard — the first save after a load is pinned to the read (objectui#11773)', () => { + it('a load of an item with a pending draft pins the first save to the version the read served', async () => { + const door = measuredDoor(); + door.savedElsewhere('object', 'acme_task', PKG, { label: 'saved earlier' }); + const { guard, ask } = guardOver(door); + await load(door, guard); + expect(await guard.save('object', 'acme_task', { label: 'A' }, DRAFT)).toBe('saved'); + expect(door.puts.map((p) => [p.ifMatch, p.ifNoneMatch])).toEqual([[v(1), null]]); + expect(ask).not.toHaveBeenCalled(); + // The draft read named the package the save writes. + expect(door.reads.some((r) => r.includes('state=draft') && r.includes(`package=${PKG}`))).toBe(true); + }); + + it('a load with no pending draft pins the first save to "no draft row" (If-None-Match: *)', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door); + await load(door, guard); + expect(await guard.save('object', 'acme_task', { label: 'A' }, DRAFT)).toBe('saved'); + await guard.save('object', 'acme_task', { label: 'B' }, DRAFT); + expect(door.puts.map((p) => [p.ifMatch, p.ifNoneMatch])).toEqual([ + [null, '*'], + [v(1), null], + ]); + }); + + it('two editors who each load and save once: the second save is refused, and the first editor\'s field survives', async () => { + const door = measuredDoor(); + const a = guardOver(door, 'cancel'); + const b = guardOver(door, 'cancel'); + await load(door, a.guard); + await load(door, b.guard); + expect(await a.guard.save('object', 'acme_task', { label: 'A', pluralLabel: 'Tasks (A2)' }, DRAFT)).toBe('saved'); + await expect(b.guard.save('object', 'acme_task', { label: 'A', description: 'B' }, DRAFT)).rejects.toThrow( + 'not saved: object/acme_task', + ); + expect(b.ask).toHaveBeenCalledWith({ type: 'object', name: 'acme_task' }); + expect(door.draftBody('object', 'acme_task', PKG)).toEqual({ label: 'A', pluralLabel: 'Tasks (A2)' }); + }); + + it('the same race with both drafts pending at load: the second save is refused on the read\'s version', async () => { + const door = measuredDoor(); + door.savedElsewhere('object', 'acme_task', PKG, { label: 'base' }); + const a = guardOver(door, 'cancel'); + const b = guardOver(door, 'cancel'); + await load(door, a.guard); + await load(door, b.guard); + await a.guard.save('object', 'acme_task', { label: 'A' }, DRAFT); + await expect(b.guard.save('object', 'acme_task', { label: 'B' }, DRAFT)).rejects.toThrow('not saved'); + expect(door.puts.map((p) => p.ifMatch)).toEqual([v(1), v(1)]); + expect(door.draftBody('object', 'acme_task', PKG)).toEqual({ label: 'A' }); + }); + + it('a read that failed pins nothing: the save goes unpinned, as before', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door); + const failing = { getDraft: vi.fn(async () => Promise.reject(new Error('network down'))) }; + const read = await readDraftForSave(failing, 'object', 'acme_task', PKG); + expect(read).toEqual({ answer: null, pin: null }); + guard.recordRead('object', 'acme_task', PKG, read.pin); + await guard.save('object', 'acme_task', { label: 'A' }, DRAFT); + expect(door.puts.map((p) => [p.ifMatch, p.ifNoneMatch])).toEqual([[null, null]]); + }); + + it('a pin belongs to its address: a read recorded for another package pins nothing here', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door); + guard.recordRead('object', 'acme_task', 'com.acme.other', { ifNoneMatch: '*' }); + await guard.save('object', 'acme_task', { label: 'A' }, DRAFT); + expect(door.puts[0]!.ifNoneMatch).toBeNull(); + }); + + it('a pin the caller carried is replaced by the held one: the two headers never leave together', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door); + await load(door, guard); + expect(await guard.save('object', 'acme_task', { label: 'A' }, { ...DRAFT, ifMatch: 'hmac-sha256:stray' })).toBe( + 'saved', + ); + expect(door.puts.map((p) => [p.ifMatch, p.ifNoneMatch])).toEqual([[null, '*']]); + }); +}); + +describe('draftPinOf — what one draft read says (objectui#11773)', () => { + it('reads the version through the spec\'s member: a token, a served null, no draft, a server without the member', () => { + expect(draftPinOf({ type: 'object', name: 'x', item: {}, version: v(7) })).toEqual({ ifMatch: v(7) }); + expect(draftPinOf({ type: 'object', name: 'x', item: {}, version: null })).toEqual({ ifNoneMatch: '*' }); + expect(draftPinOf(null)).toEqual({ ifNoneMatch: '*' }); + // A server from before the read served a version (17.x): nothing to pin. + expect(draftPinOf({ type: 'object', name: 'x', item: {} })).toBeNull(); + // Not a token by the spec's own schema: nothing to pin. + expect(draftPinOf({ type: 'object', name: 'x', item: {}, version: 7 })).toBeNull(); + }); + + it('a read that names no package of a package-bound draft serves null — which is why loads read at the save\'s package', async () => { + const door = measuredDoor(); + door.savedElsewhere('object', 'acme_task', PKG, { label: 'draft in the package' }); + const packageless = await door.client.getDraft<{ version?: unknown }>('object', 'acme_task'); + expect(packageless?.version).toBeNull(); + const atSave = await readDraftForSave(door.client, 'object', 'acme_task', PKG); + expect(atSave.pin).toEqual({ ifMatch: v(1) }); + }); +}); 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 () => { + it('with nothing recorded, a first save sends no pin; 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(door.puts.map((p) => p.ifMatch)).toEqual([null, v(1), v(2)]); 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 load(door, guard); 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([ @@ -144,26 +291,30 @@ describe('DraftVersionGuard — each draft save sends the version the last one r ]); 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' }); + expect(door.puts[2]!.ifMatch).toBe(v(2)); + expect(door.draftBody('object', 'acme_task', PKG)).toEqual({ label: 'C' }); }); - it('forget(): a buffer installed from a read holds no version, so its next save is unpinned', async () => { + it('forget(): a buffer installed with no draft read behind it holds no pin, 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]); + expect(door.puts.map((p) => [p.ifMatch, p.ifNoneMatch])).toEqual([ + [null, null], + [null, null], + ]); }); - it('a version belongs to one item: a save of another item (or package) sends none', async () => { + it('a pin belongs to one item: a save of another item (or package) sends none, and each item keeps its own', 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]); + await guard.save('object', 'acme_task', { label: 'C' }, DRAFT); + expect(door.puts.map((p) => p.ifMatch)).toEqual([null, null, null, v(1)]); }); it('a non-draft save passes straight through, unpinned and unrecorded', async () => { @@ -171,9 +322,9 @@ describe('DraftVersionGuard — each draft save sends the version the last one r 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], + expect(door.puts.map((p) => [p.draft, p.ifMatch, p.ifNoneMatch])).toEqual([ + [false, null, null], + [false, null, null], ]); }); }); @@ -183,57 +334,101 @@ describe('DraftVersionGuard — a draft saved elsewhere is not overwritten in si 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' }); + door.savedElsewhere('object', 'acme_task', PKG, { 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.draftBody('object', 'acme_task', PKG)).toEqual({ label: 'mine', description: 'theirs' }); expect(door.puts).toHaveLength(2); - // The reloaded buffer was read, not saved: its first save is unpinned. + // The caller's reload re-reads the draft: the next save is pinned to THEIR version. + await load(door, guard); await guard.save('object', 'acme_task', { label: 'mine', description: 'theirs', icon: 'x' }, DRAFT); - expect(door.puts[2]!.ifMatch).toBeNull(); + expect(door.puts[2]!.ifMatch).toBe(v(2)); }); 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' }); + door.savedElsewhere('object', 'acme_task', PKG, { 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' }); + expect(door.draftBody('object', 'acme_task', PKG)).toEqual({ label: 'theirs' }); }); - it('"overwrite" re-sends the same body without If-Match, wins, and pins the next save to its receipt', async () => { + it('"overwrite" re-sends the same body pinned to the refusal\'s currentVersion, 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' }); + const theirs = door.savedElsewhere('object', 'acme_task', PKG, { 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.puts.slice(1).map((p) => [p.ifMatch, p.body])).toEqual([ + [v(1), { label: 'mine, kept' }], + [theirs, { label: 'mine, kept' }], ]); - expect(door.draftBody('object', 'acme_task', 'com.acme.app')).toEqual({ label: 'mine, kept' }); + expect(door.draftBody('object', 'acme_task', PKG)).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')}`); + expect(door.puts[3]!.ifMatch).toBe(v(3)); }); - it('"keep editing" sends nothing, rejects, and keeps the stale version so the next save is refused again', async () => { + it('"overwrite" of a draft that a publish drained re-sends with If-None-Match: * (currentVersion null)', async () => { + const door = measuredDoor(); + const { guard } = guardOver(door, 'overwrite'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + door.publishDraft('object', 'acme_task', PKG); + + expect(await guard.save('object', 'acme_task', { label: 'after publish' }, DRAFT)).toBe('saved'); + expect(door.puts.slice(1).map((p) => [p.ifMatch, p.ifNoneMatch])).toEqual([ + [v(1), null], + [null, '*'], + ]); + }); + + it('a save landing between the refusal and the confirmed overwrite is refused again and asked about, never overwritten', 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' }); + door.savedElsewhere('object', 'acme_task', PKG, { label: 'theirs' }); + // The third editor saves while the first one's dialog is open; the author + // overwrites, is asked again about the third save, and keeps editing. + ask.mockImplementationOnce(async () => { + door.savedElsewhere('object', 'acme_task', PKG, { label: 'a third editor' }); + return 'overwrite'; + }); + + await expect(guard.save('object', 'acme_task', { label: 'mine, kept' }, DRAFT)).rejects.toThrow('not saved'); + expect(ask).toHaveBeenCalledTimes(2); + expect(door.draftBody('object', 'acme_task', PKG)).toEqual({ label: 'a third editor' }); + expect(door.puts.slice(1).map((p) => p.ifMatch)).toEqual([v(1), v(2)]); + }); + + it('control: a refusal without currentVersion (a server before the field) overwrites unpinned, as it always did', async () => { + const door = measuredDoor({ conflictCarriesCurrentVersion: false }); + const { guard } = guardOver(door, 'overwrite'); + await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); + door.savedElsewhere('object', 'acme_task', PKG, { label: 'theirs' }); + expect(await guard.save('object', 'acme_task', { label: 'mine, kept' }, DRAFT)).toBe('saved'); + expect(door.puts.slice(1).map((p) => [p.ifMatch, p.ifNoneMatch])).toEqual([ + [v(1), null], + [null, null], + ]); + }); + + it('"keep editing" sends nothing, rejects, and keeps the stale pin 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', PKG, { label: 'theirs' }); await expect(guard.save('object', 'acme_task', { label: 'stale' }, DRAFT)).rejects.toThrow( 'not saved: object/acme_task', @@ -242,27 +437,31 @@ describe('DraftVersionGuard — a draft saved elsewhere is not overwritten in si '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' }); + expect(door.puts.slice(1).every((p) => p.ifMatch === v(1))).toBe(true); + expect(door.draftBody('object', 'acme_task', PKG)).toEqual({ label: 'theirs' }); }); - it('control: once a publish dropped the draft, ANY version is refused — which is why an install must forget()', async () => { + it('once a publish dropped the draft, a held version is refused — and the publish\'s re-read pins "no draft row" instead', 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'); + door.publishDraft('object', 'acme_task', PKG); 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(); + // What every editor does when the publish reload installs the buffer. + await load(door, guard); 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]); + expect(door.puts.map((p) => [p.ifMatch, p.ifNoneMatch])).toEqual([ + [null, null], + [v(1), null], + [null, '*'], + ]); }); }); 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 () => { + it('DESTRUCTIVE_CHANGE passes through untouched, and the forced retry carries the same pin', async () => { const door = measuredDoor(); const { guard, ask } = guardOver(door, 'overwrite'); await guard.save('object', 'acme_task', { label: 'mine' }, DRAFT); @@ -278,10 +477,9 @@ describe('DraftVersionGuard — the door\'s two 409s stay apart (objectui#11773) 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], + [false, v(1)], + [true, v(1)], ]); }); @@ -289,7 +487,7 @@ describe('DraftVersionGuard — the door\'s two 409s stay apart (objectui#11773) 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' }); + door.savedElsewhere('object', 'acme_task', PKG, { label: 'theirs' }); const raw = await door.client .save('object', 'acme_task', { label: 'x' }, { ...DRAFT, ifMatch: 'hmac-sha256:stale' }) .catch((e: unknown) => e); diff --git a/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.tsx b/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.tsx index 866eec3fa7..600f21ea55 100644 --- a/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.tsx +++ b/packages/app-shell/src/views/metadata-admin/DraftConflictDialog.tsx @@ -6,54 +6,66 @@ * 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. + * built on. The `/meta` PUT door honours ADR-0008's precondition on a + * `?mode=draft` write, and `MetadataClient.save` sends it from its options: + * `ifMatch` as `If-Match`, `ifNoneMatch` as `If-None-Match: *`. * - * ## Where the version comes from (measured, not read off a docblock) + * ## What the door serves and refuses (`@objectstack/*` 18, measured) * - * 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): + * Read off the spec this package installs (`GetMetaItemResponseSchema`, + * `MetadataConflictErrorSchema`) and measured against the published server of + * the same release (the readings are on the pull request): * - * - 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). + * - The draft READ (`GET …?state=draft&package=P`) serves `version`: the token + * of the draft row a draft save at the SAME address (`?package=P`) compares + * against, or `null` when that address holds none. With no draft at all it + * answers 404, which `getDraft` resolves as `null`. + * - ⚠️ The address is the read's, not the served row's. A read that names no + * package serves a package-bound draft's body and `version: null`, so its + * answer says nothing about a save that names the package. A load therefore + * reads at the address its saves write ({@link readDraftForSave}). + * - A draft SAVE answers `{ success, version, seq, state, message }`; that + * `version` is the same token the next read serves. + * - `If-Match: ` that is not the current draft's token, and + * `If-None-Match: *` while a draft row exists, are refused + * `409 METADATA_CONFLICT`, whose body carries the current token as + * `currentVersion` (`null` when no draft row is there). The pair together is + * refused 400. * - `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: + * The pin 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 pin 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. + * 1. A draft save sends the pin this guard holds for that same item (type, + * name, package) — `If-Match` with a version, or `If-None-Match: *` for "no + * draft row" — and nothing otherwise. Never both. * 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. + * 3. A buffer installed from a load holds what that load's draft read says + * ({@link draftPinOf}): the caller calls `recordRead` with the pin + * {@link readDraftForSave} returned, where it installs the buffer. A read + * that failed, or a server that serves no `version`, pins nothing; a buffer + * installed with no draft read behind it (a reset, a discard) holds nothing + * either: the caller says so with `forget()`. A read-back of the guard's + * OWN save is not 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). + * after a confirmation (re-sent pinned to the refusal's `currentVersion`, + * parsed by the spec's `MetadataConflictErrorSchema`, so a third save that + * lands in between is refused again rather than overwritten), or keep + * editing (nothing is saved, and the stale pin 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. + * A create, with nothing read, calls the client directly with + * `ifNoneMatch: '*'`: the door refuses it when the name already holds a draft, + * instead of writing the new skeleton over it. */ import * as React from 'react'; @@ -69,6 +81,7 @@ import { cn, } from '@object-ui/components'; import type { MetadataClient, MetadataClientSaveOptions } from '@object-ui/data-objectstack'; +import { GetMetaItemResponseSchema, MetadataConflictErrorSchema } from '@objectstack/spec/api'; import { errorCodeIs } from '@object-ui/types'; import { t, tFormat, translateMetadataType, useMetadataLocale } from './i18n.js'; @@ -92,9 +105,72 @@ 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; +/** + * The precondition a guarded draft save is sent with: exactly one of the two + * headers, by construction never both. + */ +export type DraftPin = { readonly ifMatch: string } | { readonly ifNoneMatch: '*' }; + +/** + * A served `version` — a token, or `null` for "no row at this address" — as the + * pin a save to that address sends. An empty token pins nothing: the client + * sends no `If-Match` for it. + */ +function pinOfVersion(version: string | null): DraftPin | null { + if (version === null) return { ifNoneMatch: '*' }; + return version.length > 0 ? { ifMatch: version } : null; +} + +/** + * What a load's DRAFT read (`MetadataClient.getDraft`, at the address the + * buffer's saves write) says the next draft save is pinned with: + * + * - `null` — no pending draft (the drafts ledger lists none, or the read + * answered 404 `NO_DRAFT`): `If-None-Match: *`; + * - the envelope's `version`, read through `GetMetaItemResponseSchema`'s own + * member: a token pins `If-Match`, a served `null` pins `If-None-Match: *`; + * - an envelope that carries no `version` (a server from before the read + * served one) pins nothing, and the save goes unpinned as it always did. + */ +export function draftPinOf(answer: unknown): DraftPin | null { + if (answer === null) return { ifNoneMatch: '*' }; + if (!answer || typeof answer !== 'object' || !('version' in answer)) return null; + const version = GetMetaItemResponseSchema.shape.version.safeParse((answer as { version: unknown }).version); + return version.success && version.data !== undefined ? pinOfVersion(version.data) : null; +} + +/** + * The pin an "overwrite" re-sends with: the refused save's `currentVersion`, + * parsed by the spec's `MetadataConflictErrorSchema` off the body the client + * kept. A body that does not carry one (a server from before the field) + * pins nothing, and the overwrite is sent unpinned, as it was before. + */ +function pinOfConflict(err: unknown): DraftPin | null { + const body = MetadataConflictErrorSchema.safeParse((err as { body?: unknown } | null | undefined)?.body); + return body.success ? pinOfVersion(body.data.currentVersion) : null; +} + +/** + * A load's draft read at `packageId` — the address the buffer's draft saves + * write — with the pin it serves for that address. Read and pin come from one + * call so a load cannot pin a save to a read of another address (see the + * module header). A read that fails resolves `answer: null`, as the loads' + * `.catch(() => null)` always did, and pins nothing: a failure says nothing + * about the row. Record the pin with `recordRead` where the buffer is + * installed, not here: a load that is cancelled installs nothing. + */ +export async function readDraftForSave( + client: Pick, + type: string, + name: string, + packageId: string | null | undefined, +): Promise<{ answer: T | null; pin: DraftPin | null }> { + try { + const answer = await client.getDraft(type, name, packageId ? { packageId } : {}); + return { answer, pin: draftPinOf(answer) }; + } catch { + return { answer: null, pin: null }; + } } function itemKey(type: string, name: string, packageId: string | null | undefined): string { @@ -106,6 +182,13 @@ function receiptVersion(receipt: unknown): string | null { return typeof version === 'string' && version.length > 0 ? version : null; } +/** `options` sent with `pin` in place of whatever precondition they carried. */ +function pinned(options: MetadataClientSaveOptions, pin: DraftPin | null): MetadataClientSaveOptions { + if (!pin) return options; + const { ifMatch: _ifMatch, ifNoneMatch: _ifNoneMatch, ...rest } = options; + return { ...rest, ...pin }; +} + export interface DraftVersionGuardDeps { /** The client to save through, read at the moment of the save. */ client: () => Pick; @@ -118,12 +201,17 @@ export interface DraftVersionGuardDeps { } /** - * The React-free half of the guard: the version held for one buffer, the + * The React-free half of the guard: the pins 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; + /** + * The pin held per item (type, name, package) this buffer saves. One entry in + * the usual case; a surface whose one buffer switches between items (the + * hook panel) keeps each item's own. + */ + private readonly held = new Map(); private tail: Promise | null = null; /** Bumped by a reload: a save queued before it carries the replaced buffer. */ private epoch = 0; @@ -135,14 +223,23 @@ export class DraftVersionGuard { 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 pin held, for the item `save` would address (tests read it). */ + heldFor(type: string, name: string, packageId?: string | null): DraftPin | null { + return this.held.get(itemKey(type, name, packageId)) ?? null; } - /** The buffer was installed from a server read: no version describes it. */ + /** The buffer was installed with no draft read behind it: nothing describes it. */ forget = (): void => { - this.held = null; + this.held.clear(); + }; + + /** + * The buffer was installed from a load whose draft read, at the address the + * buffer's saves write (`type`, `name`, `packageId`), served `pin` — what + * {@link readDraftForSave} returned. `null` pins nothing, as `forget()`. + */ + recordRead = (type: string, name: string, packageId: string | null | undefined, pin: DraftPin | null): void => { + this.pin(itemKey(type, name, packageId), pin); }; /** @@ -184,45 +281,53 @@ export class DraftVersionGuard { // 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; + let pin = this.held.get(key) ?? 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); + for (;;) { + try { + const receipt = await client.save(type, name, item, pinned(options, pin)); 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.clear(); + this.epoch += 1; + this.deps.reload(); + return 'reloaded'; + } + if (choice !== 'overwrite') throw this.deps.notSaved(conflict); + // The author has seen that the draft moved and chose this buffer over + // the version the refusal names, so the re-send is pinned to THAT + // version: a save landing between the refusal and the confirmation is + // refused again and asked about, never overwritten. Held, too, so a + // refusal of the re-send for another reason (a destructive change the + // caller confirms) retries over the same version. + pin = pinOfConflict(err); + this.pin(key, pin); } - throw this.deps.notSaved(conflict); } } private hold(key: string, receipt: unknown): void { const version = receiptVersion(receipt); - this.held = version ? { key, version } : null; + this.pin(key, version ? { ifMatch: version } : null); + } + + private pin(key: string, pin: DraftPin | null): void { + if (pin) this.held.set(key, pin); + else this.held.delete(key); } } 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). */ + /** Call where the buffer is installed from a load's draft read (rule 3 above). */ + recordRead: DraftVersionGuard['recordRead']; + /** Call where the buffer is installed with no draft read behind it (rule 3 above). */ forget: () => void; /** The conflict dialog; render it once, anywhere in the caller's tree. */ dialog: React.ReactElement; @@ -276,6 +381,7 @@ export function useDraftSaveGuard(client: Pick, onReload }, []); return { save: guard.save, + recordRead: guard.recordRead, forget: guard.forget, dialog: , }; diff --git a/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx index 3132605e16..d98212df93 100644 --- a/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx +++ b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx @@ -76,7 +76,7 @@ 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 { readDraftForSave, useDraftSaveGuard } from './DraftConflictDialog.js'; // objectui#11787 — the package door autosaves through the pillars' own autosave. import { useDraftAutoSave, type DraftSend } from '../studio-design/useDraftAutoSave.js'; import { useDisplayLocale } from '@object-ui/i18n'; @@ -478,12 +478,14 @@ 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. + // objectui#11773 — the pin of the row the package door's draft was built on + // (the load's draft read, then each save's receipt), sent 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, + recordRead: recordSetRead, forget: forgetSetVersion, dialog: draftConflictDialog, } = useDraftSaveGuard(client, reloadSet); @@ -577,9 +579,12 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, // metadata, so surface the PENDING draft if one exists — otherwise a // just-saved-not-yet-published edit would appear lost on reopen. Draft // reads return the `{ type, name, item }` envelope; `null` = no draft. - const draftRead = packageId - ? client.getDraft<{ item?: PermissionSetDraft } | PermissionSetDraft>(type, name, { packageId }).catch(() => null) + // objectui#11773 — read at the package door's save address, with the + // pin it serves for that address (the package door's draft save). + const draftReadForSave = packageId + ? readDraftForSave<{ item?: PermissionSetDraft } | PermissionSetDraft>(client, type, name, packageId) : Promise.resolve(null); + const draftRead = draftReadForSave.then((read) => read?.answer ?? null); const readLayered = () => client.layered(type, name).catch(() => null); // objectui#11799 — a set "+ New" created and nobody published has a // draft and no layer, and `GET …/layers` answers 404 for it. Under the @@ -597,7 +602,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, packageId && type === 'permission' && publishedSets ? publishedSets.then((rows) => !(rows || []).some((r) => r?.name === name), () => false) : Promise.resolve(false); - const [lay, objList, pendingDraft] = await Promise.all([ + const [lay, objList, pendingRead] = await Promise.all([ packageId ? neverPublished.then((unlisted) => unlisted ? draftRead.then((d) => (d ? null : readLayered())) : readLayered(), @@ -606,9 +611,10 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, // In package scope, list only the objects this package declares // (ADR-0086 P0) — otherwise the whole environment leaks into the panel. client.list('object', packageId ? { packageId } : {}).catch(() => []), - draftRead, + draftReadForSave, ]); if (cancelled) return; + const pendingDraft = pendingRead?.answer ?? null; // ARTIFACT tier input (objectui#4518) — the `code` layer, and its // `provenance` (objectui#4526), of the SAME envelope the display // baseline comes from, so the writability verdict and the body on @@ -665,8 +671,11 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, } else { resetDraftBaseline(full); } - // objectui#11773 — a read serves no version: the next save is unpinned. - forgetSetVersion(); + // objectui#11773 — the package door's next draft save is pinned to what + // the draft read served. The environment door reads no draft and its + // live write is not pinned. + if (pendingRead && packageId) recordSetRead(type, name, packageId, pendingRead.pin); + else forgetSetVersion(); } catch (err: any) { setError(err?.message ?? String(err)); } finally { @@ -676,7 +685,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, return () => { cancelled = true; }; - }, [client, type, name, packageId, publishNonce, resetDraftBaseline, reloadNonce, forgetSetVersion]); + }, [client, type, name, packageId, publishNonce, resetDraftBaseline, reloadNonce, recordSetRead, forgetSetVersion]); /* ── Lazy-load fields when an object is expanded ─────────── */ async function ensureFields(objectName: string) { diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.createFirstWritePin-11773.test.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.createFirstWritePin-11773.test.tsx new file mode 100644 index 0000000000..fa9a6cd078 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.createFirstWritePin-11773.test.tsx @@ -0,0 +1,180 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — the metadata editor's CREATE is the first-write pin. + * + * A create has read nothing, so it has no version to send. The `/meta` door + * of `@objectstack/*` 18 takes `If-None-Match: *` for that case — "save only + * where no draft row of this name is" — and refuses it `409 METADATA_CONFLICT` + * where one is, instead of writing the new skeleton over it. It also reads the + * pin as a create, which picks the create remedy on a sealed name + * (objectstack-ai/objectstack#22591). Before this, `doSave` sent a create with + * no precondition at all, which the door reads as an edit. + * + * The REAL page over a REAL `MetadataClient`, whose transport is an in-memory + * server answering as the published 18.0.0-next.0 door was measured to: the + * request headers it saw are recorded. The `doc` type's editor stands in for + * every type (the create flow is the page's own), as in + * `ResourceEditPage.layersUnsaved-11799.test.tsx`. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom'; +import { z } from 'zod'; +import { ComponentRegistry } from '@object-ui/core'; +import { DocSchema } from '@objectstack/spec/system'; +import { MetadataClient } from '@object-ui/data-objectstack'; + +const server = { + drafts: new Map; version: string }>(), + puts: [] as Array<{ name: string; ifMatch: string | null; ifNoneMatch: string | null; status: number }>, + seq: 0, +}; + +const json = (status: number, body: unknown) => + new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } }); + +async function serve(input: RequestInfo | URL, init?: RequestInit): Promise { + const method = (init?.method ?? 'GET').toUpperCase(); + const url = new URL(String(input), 'http://console.test'); + const meta = url.pathname.match(/^\/api\/v1\/meta\/(.+)$/); + if (!meta) return json(200, []); + const seg = meta[1].split('/').map(decodeURIComponent); + const key = `${seg[0]}/${seg[1]}`; + if (seg[0] === '_drafts') { + return json(200, { drafts: [...server.drafts.keys()].map((k) => ({ type: k.split('/')[0], name: k.split('/')[1] })) }); + } + if (seg.length === 1) return json(200, { items: [] }); + if (seg.length === 3) return json(404, { error: { code: 'NOT_FOUND', message: 'absent' } }); + if (method === 'PUT') { + const headers = new Headers(init?.headers); + const ifMatch = headers.get('If-Match'); + const ifNoneMatch = headers.get('If-None-Match'); + const head = server.drafts.get(key)?.version ?? null; + const refused = + (ifMatch !== null && ifMatch !== head) || (ifNoneMatch !== null && head !== null); + const status = ifMatch !== null && ifNoneMatch !== null ? 400 : refused ? 409 : 200; + server.puts.push({ name: seg[1]!, ifMatch, ifNoneMatch, status }); + if (status === 400) return json(400, { error: { code: 'VALIDATION_ERROR', message: 'Send If-Match or If-None-Match, not both.' } }); + if (status === 409) { + return json(409, { + error: `${key} has been modified since you loaded it. Expected parent null but current is ${head}.`, + code: 'METADATA_CONFLICT', + currentVersion: head, + }); + } + server.seq += 1; + const version = `hmac-sha256:v${server.seq}`; + server.drafts.set(key, { body: JSON.parse(String(init?.body ?? '{}')) as Record, version }); + return json(200, { success: true, version, seq: server.seq, state: 'draft', message: `Saved ${key}` }); + } + const row = url.searchParams.get('state') === 'draft' ? server.drafts.get(key) : undefined; + if (!row) return json(404, { error: { code: 'NO_DRAFT', message: 'absent' } }); + return json(200, { type: seg[0], name: seg[1], item: { ...row.body, _draft: true }, version: row.version }); +} + +const client = vi.hoisted(() => ({ current: null as unknown })); + +vi.mock('./useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => client.current, + useMetadataTypes: () => ({ + loading: false, + entries: [ + { type: 'doc', label: 'Documentation', domain: 'system', allowEnvironmentOverlay: false, allowRuntimeCreate: true, schema: DOC_JSON_SCHEMA }, + ], + }), + }; +}); + +import { MetadataResourceEditPage } from './ResourceEditPage'; +import { registerDefaultMetadataSchemas } from './default-schemas'; +import { registerBuiltinPreviews } from './previews'; + +const DOC_JSON_SCHEMA = z.toJSONSchema(DocSchema as unknown as z.ZodType) as Record; +client.current = new MetadataClient({ baseUrl: '', fetch: vi.fn(serve) as unknown as typeof fetch }); + +registerDefaultMetadataSchemas(); +registerBuiltinPreviews(); + +const STUB_NS = 'create-first-write-pin-11773'; +function StubMarkdown({ schema }: { schema: { content?: string } }) { + return
{schema.content}
; +} +beforeAll(() => { + ComponentRegistry.register('markdown', StubMarkdown as never, { namespace: STUB_NS }); +}); +afterAll(() => { + ComponentRegistry.unregister('markdown', STUB_NS); +}); + +beforeEach(() => { + server.drafts.clear(); + server.puts = []; + server.seq = 0; + window.localStorage.setItem('metadata-admin:autosave', '0'); +}); +afterEach(() => { + cleanup(); + window.localStorage.removeItem('metadata-admin:autosave'); +}); + +function Where() { + return
{useLocation().pathname}
; +} + +const source = () => screen.findByRole('textbox', { name: 'Markdown source' }, { timeout: 8000 }); +const saveButton = () => screen.getByRole('button', { name: /Save \(⌘S\)/ }); + +/** Open the create form, name the doc "New Guide", give it a body, and press Save. */ +async function createNewGuide() { + render( + + + + + + + } + /> + } /> + + , + ); + const label = await waitFor(() => { + const el = document.getElementById('mdf-label') as HTMLInputElement | null; + expect(el).not.toBeNull(); + return el!; + }, { timeout: 8000 }); + fireEvent.change(label, { target: { value: 'New Guide' } }); + await waitFor(() => expect((document.getElementById('mdf-name') as HTMLInputElement).value).toBe('new_guide')); + fireEvent.change(await source(), { target: { value: '# New' } }); + fireEvent.click(saveButton()); +} + +describe('MetadataResourceEditPage — a create sends If-None-Match: * (objectui#11773)', () => { + it('the create is sent with the first-write pin and no If-Match, and lands', async () => { + await createNewGuide(); + await waitFor(() => expect(screen.getByTestId('where')).toHaveTextContent('/metadata/doc/new_guide'), { timeout: 8000 }); + expect(server.puts).toEqual([{ name: 'new_guide', ifMatch: null, ifNoneMatch: '*', status: 200 }]); + expect(server.drafts.get('doc/new_guide')?.body).toMatchObject({ name: 'new_guide', content: '# New' }); + }); + + it('a create over a name that already holds a draft is refused, and that draft is not written over', async () => { + server.drafts.set('doc/new_guide', { body: { name: 'new_guide', content: '# Someone else\'s' }, version: 'hmac-sha256:theirs' }); + await createNewGuide(); + await waitFor(() => expect(server.puts).toHaveLength(1), { timeout: 8000 }); + expect(server.puts[0]).toEqual({ name: 'new_guide', ifMatch: null, ifNoneMatch: '*', status: 409 }); + expect(server.drafts.get('doc/new_guide')?.body).toEqual({ name: 'new_guide', content: '# Someone else\'s' }); + // The page stays on the create form: nothing was created. + expect(screen.getByTestId('where')).toHaveTextContent('/metadata/doc/new'); + }); +}); 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 index b37e1604f8..a80d36d813 100644 --- 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 @@ -12,10 +12,13 @@ * - `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. + * The door double answers the way the published 18.0.0-next.0 door was + * measured to (readings on the pull request): the draft read serves the + * draft's `version` (no draft: `null` from `getDraft`), a stale `If-Match` or + * an `If-None-Match: *` over an existing draft is `409 METADATA_CONFLICT` with + * `currentVersion`. 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'; @@ -40,6 +43,7 @@ interface SaveCall { body: Record; force: boolean; ifMatch: string | undefined; + ifNoneMatch: string | undefined; } const door = { @@ -73,17 +77,22 @@ 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)), + // The draft read serves the body and its version; no draft is `null`. + getDraft: vi.fn(async () => + door.draft + ? { type: 'page', name: 'home', item: JSON.parse(JSON.stringify(door.draft.body)), version: door.draft.version } + : 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 }) => { + save: vi.fn(async (_type: string, _name: string, item: unknown, options?: { force?: boolean; ifMatch?: string; ifNoneMatch?: string }) => { const body = JSON.parse(JSON.stringify(item)) as Record; const ifMatch = options?.ifMatch; + const ifNoneMatch = options?.ifNoneMatch; const force = !!options?.force; - door.saves.push({ body, force, ifMatch }); + door.saves.push({ body, force, ifMatch, ifNoneMatch }); 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.", @@ -92,10 +101,11 @@ const mockClient = { }); } const head = door.draft?.version ?? null; - if (ifMatch !== undefined && ifMatch !== head) { + if ((ifMatch !== undefined && ifMatch !== head) || (ifNoneMatch !== undefined && head !== null)) { 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', + currentVersion: head, }); } const version = writeDraft(body); @@ -164,9 +174,10 @@ async function patchAndSave(label: string): Promise { } 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 () => { + it('the first autosave is pinned to the load (no draft: If-None-Match); each later one sends the version the one before it received', async () => { renderEditor(); - expect((await patchAndSave('One')).ifMatch).toBeUndefined(); + const first = await patchAndSave('One'); + expect([first.ifMatch, first.ifNoneMatch]).toEqual([undefined, '*']); 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 }); @@ -189,10 +200,52 @@ describe('MetadataResourceEditPage — draft saves carry their version (objectui 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. + // The reload re-read the draft, so the next save is pinned to THEIR + // version and starts from their body. await waitFor(() => expect(mockClient.getDraft.mock.calls.length).toBeGreaterThanOrEqual(3), { timeout: 8000 }); - expect((await patchAndSave('Three')).ifMatch).toBeUndefined(); + expect((await patchAndSave('Three')).ifMatch).toBe('hmac-sha256:v2'); + }); + + it('a load of an item with a pending draft: the first save sends If-Match with the version the read served', async () => { + const theirs = writeDraft({ ...PAGE, label: 'Pending' }); + renderEditor(); + const first = await patchAndSave('One'); + expect([first.ifMatch, first.ifNoneMatch]).toEqual([theirs, undefined]); + // The draft was read at the package the save writes. + expect(mockClient.getDraft).toHaveBeenCalledWith('page', 'home', { packageId: 'com.acme.app' }); + expect(screen.queryByTestId('draft-conflict-dialog')).toBeNull(); + }); + + it('two editors who each load and save once: the later first save is refused, and the earlier draft survives', async () => { + renderEditor(); + await screen.findByRole('button', { name: 'label One' }, { timeout: 8000 }); + // The other editor, who opened the page at the same time, saves first. + writeDraft({ ...PAGE, label: 'Theirs', description: 'kept' }); + + const mine = await patchAndSave('One'); + expect([mine.ifMatch, mine.ifNoneMatch]).toEqual([undefined, '*']); + expect(await screen.findByTestId('draft-conflict-dialog', undefined, { timeout: 8000 })).toHaveTextContent('home'); + expect(door.draft?.body).toMatchObject({ label: 'Theirs', description: 'kept' }); + }); + + it('"overwrite" after a 409 re-sends pinned to the refusal\'s currentVersion, and the next save to its receipt', async () => { + renderEditor(); + await patchAndSave('One'); + await waitFor(() => expect(door.draft?.version).toBe('hmac-sha256:v1'), { timeout: 8000 }); + const theirs = writeDraft({ ...PAGE, label: 'Theirs' }); + + await patchAndSave('Two'); + await screen.findByTestId('draft-conflict-dialog', undefined, { timeout: 8000 }); + fireEvent.click(screen.getByTestId('draft-conflict-overwrite')); + const before = door.saves.length; + fireEvent.click(await screen.findByTestId('draft-conflict-overwrite-confirm')); + await waitFor(() => expect(door.saves.length).toBe(before + 1), { timeout: 8000 }); + const overwrite = door.saves[door.saves.length - 1]!; + expect([overwrite.ifMatch, overwrite.ifNoneMatch, overwrite.body.label]).toEqual([theirs, undefined, 'Two']); + await waitFor(() => expect(door.draft?.body.label).toBe('Two'), { timeout: 8000 }); + const overwriteReceipt = door.draft?.version; + + expect((await patchAndSave('Three')).ifMatch).toBe(overwriteReceipt); }); it('control: a destructive change still opens its own confirmation, and the forced retry keeps the version', async () => { diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index 6057253423..2e7c7ed797 100644 --- a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -113,7 +113,7 @@ import { type RichMetadataTypeEntry, } from './useMetadata.js'; // objectui#11773 — the draft save's optimistic-concurrency guard and its dialog. -import { useDraftSaveGuard } from './DraftConflictDialog.js'; +import { readDraftForSave, useDraftSaveGuard } from './DraftConflictDialog.js'; import { getMetadataResource, resolveResourceConfig, @@ -722,12 +722,14 @@ 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. + // objectui#11773 — the pin of the row the editor's draft was built on (the + // load's draft read, then each save's receipt), sent 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, + recordRead: recordDraftRead, forget: forgetDraftVersion, dialog: draftConflictDialog, } = useDraftSaveGuard(client, reloadFromServer); @@ -1091,14 +1093,19 @@ function MetadataResourceEditPageImpl({ (async () => { try { const scope = ownerPackageId ? { packageId: ownerPackageId } : {}; - const [lay, draftResp] = await Promise.all([ + // objectui#11773 — the draft is read at the address `doSave` writes + // (`readActivePackageBinding`, the folded `?package=`), so the version + // it serves is the one that save compares against. + const draftPackage = readActivePackageBinding(); + const [lay, draftRead] = await Promise.all([ client.layered(type, name, scope), // Draft reads are best-effort — a 404/error must not block // the page; readers without overlay-write permission still // see the published item. - client.getDraft(type, name, scope).catch(() => null), + readDraftForSave(client, type, name, draftPackage), ]); if (cancelled) return; + const draftResp = draftRead.answer; setLayered(lay); // Surface server-computed load-time validation errors as inline // SchemaForm issues — operators see what's wrong with the @@ -1138,8 +1145,9 @@ 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(); + // objectui#11773 — the next save is pinned to what the draft read + // served: its version, or "no draft row" when none was pending. + recordDraftRead(type, name, draftPackage, draftRead.pin); setHasDraft(!!draftReal); setLoading(false); } catch (err: any) { @@ -1163,7 +1171,7 @@ function MetadataResourceEditPageImpl({ return () => { cancelled = true; }; - }, [client, type, name, ownerPackageId, createMode, reloadKey, locale, forgetDraftVersion]); + }, [client, type, name, ownerPackageId, createMode, reloadKey, locale, recordDraftRead]); // Lazy-load references the first time the References sheet opens. // @@ -1638,11 +1646,12 @@ function MetadataResourceEditPageImpl({ 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. + // objectui#11773 — a create, with nothing read, is the first-write pin + // (`If-None-Match: *`): the door refuses it where the name already holds + // a draft instead of writing over it, and reads it as a create. A save of + // the item this editor loaded sends the pin its load or last save held. if (createMode) { - await client.save(type, savedName, itemToSave, saveOptions); + await client.save(type, savedName, itemToSave, { ...saveOptions, ifNoneMatch: '*' }); } else if ((await saveDraftVersioned(type, savedName, itemToSave, saveOptions)) === 'reloaded') { // The author chose the saved version; the load effect replaces the draft. return; @@ -1803,7 +1812,8 @@ function MetadataResourceEditPageImpl({ setError(null); try { await client.reset(type, name); - // objectui#11773 — the buffer below is re-read, so no version describes it. + // objectui#11773 — the buffer below is re-read from the layers, with no + // draft read behind it, so nothing pins its next save. forgetDraftVersion(); if (isResetSemantic) { const lay = await client.layered(type, name); @@ -1845,10 +1855,13 @@ function MetadataResourceEditPageImpl({ await client.publish(type, name, { ...(activePackage ? { packageId: activePackage } : {}), }); - const [lay, draftResp] = await Promise.all([ + // objectui#11773 — the draft is re-read at the address the next save + // writes, so its answer pins that save (see the load effect). + const [lay, draftRead] = await Promise.all([ client.layered(type, name), - client.getDraft(type, name).catch(() => null), + readDraftForSave(client, type, name, activePackage), ]); + const draftResp = draftRead.answer; setLayered(lay); const draftReal = extractDraftBody(draftResp); setHasDraft(!!draftReal); @@ -1863,8 +1876,10 @@ 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(); + // objectui#11773 — the publish dropped the draft the held version named; + // the re-read says what the next save is pinned to (normally "no draft + // row"). + recordDraftRead(type, name, activePackage, draftRead.pin); } catch (err: any) { setError(err?.message ?? String(err)); } finally { @@ -1882,7 +1897,10 @@ function MetadataResourceEditPageImpl({ setError(null); try { await client.reset(type, name, { state: 'draft' }); - // objectui#11773 — the discard dropped the draft the version named. + // objectui#11773 — the discard dropped the draft the held version named, + // and no draft read follows it (the discard names no package, so "no + // draft row" is not known at the save's address): nothing pins the next + // save. forgetDraftVersion(); // objectui#11799 — a discard makes no layer: an item this page loaded // with no layer still has none, and `/layers` would answer 404. diff --git a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.firstSavePinned-11773.test.tsx b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.firstSavePinned-11773.test.tsx new file mode 100644 index 0000000000..d2291e1518 --- /dev/null +++ b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.firstSavePinned-11773.test.tsx @@ -0,0 +1,158 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — a hook's first draft save after the panel's load is pinned + * to what that load read for it. + * + * The panel installs its buffer from a LIST, not from one item read: the + * package's published hooks plus each draft this package's drafts ledger + * lists. So the list load records one pin per hook (`recordRead`): + * + * - a hook with a pending draft: the `version` its draft read served, read at + * the package the save writes; + * - a hook the ledger does not list: "no draft row", `If-None-Match: *`; + * - an unreadable ledger says nothing, and pins nothing (the save goes + * unpinned, as before). + * + * The door double answers as the published 18.0.0-next.0 door was measured to + * (readings on the pull request); the guard keeps each hook's own pin, so a + * hook saved, left and re-opened is still pinned to its last receipt. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; + +const PKG = 'com.example.showcase'; +const guard = { name: 'guard_hook', label: 'Guard', object: 'invoice', events: ['beforeInsert'], handler: 'guard_fn' }; +const audit = { name: 'audit_hook', label: 'Audit', object: 'invoice', events: ['afterInsert'], handler: 'audit_fn' }; + +interface Sent { + name: string; + ifMatch: string | undefined; + ifNoneMatch: string | undefined; +} + +const door = { + drafts: new Map; version: string }>(), + ledgerFails: false, + sent: [] as Sent[], + seq: 0, +}; + +const mockClient = { + list: vi.fn(async () => [guard, audit]), + listDrafts: vi.fn(async () => { + if (door.ledgerFails) throw new Error('403 drafts ledger'); + return [...door.drafts.keys()].map((name) => ({ type: 'hook', name })); + }), + getDraft: vi.fn(async (_type: string, name: string, _options?: { packageId?: string }) => { + const row = door.drafts.get(name); + return row ? { type: 'hook', name, item: JSON.parse(JSON.stringify(row.body)), version: row.version } : null; + }), + get: vi.fn(async () => null), + withPreviewDrafts() { + return this; + }, + save: vi.fn( + async (_type: string, name: string, body: Record, options?: { ifMatch?: string; ifNoneMatch?: string }) => { + door.sent.push({ name, ifMatch: options?.ifMatch, ifNoneMatch: options?.ifNoneMatch }); + door.seq += 1; + const version = `hmac-sha256:v${door.seq}`; + door.drafts.set(name, { body: JSON.parse(JSON.stringify(body)), version }); + return { success: true, version, seq: door.seq, state: 'draft' }; + }, + ), +}; + +vi.mock('../metadata-admin/useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useMetadataClient: () => mockClient }; +}); + +import { ObjectHooksPanel } from './ObjectHooksPanel'; +import { registerBuiltinInspectors } from '../metadata-admin/inspectors'; +import { __setCelFormulaLoader } from '../metadata-admin/celAuthoring'; + +registerBuiltinInspectors(); + +beforeEach(() => { + door.drafts.clear(); + door.ledgerFails = false; + door.sent.length = 0; + door.seq = 0; + for (const fn of [mockClient.list, mockClient.listDrafts, mockClient.getDraft, mockClient.save]) fn.mockClear(); + __setCelFormulaLoader(() => + Promise.resolve({ + validateExpression: () => ({ ok: true, errors: [], warnings: [] }), + introspectScope: () => ({ fields: ['status'], roots: ['record'], functions: ['has'] }), + inferExpressionType: () => 'boolean' as const, + }), + ); +}); +afterEach(() => { + cleanup(); + __setCelFormulaLoader(undefined); +}); + +const AUTOSAVE = { timeout: 4000 }; + +/** Open a hook by its label and send one edit through the autosave. */ +async function editAndSave(label: string, condition: string): Promise { + fireEvent.click(await screen.findByText(label)); + fireEvent.click(await screen.findByText('Expression')); + const box = screen.getAllByRole('combobox').find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement; + const before = door.sent.length; + fireEvent.change(box, { target: { value: condition } }); + await waitFor(() => expect(door.sent.length).toBe(before + 1), AUTOSAVE); + await waitFor(() => expect(screen.getByTestId('hooks-saved-at')).toBeInTheDocument()); + return door.sent[door.sent.length - 1]!; +} + +function openPanel() { + render(); +} + +describe('ObjectHooksPanel — the first save after the list load is pinned (objectui#11773)', () => { + it('a published hook this package holds no draft of: the first save sends If-None-Match: *', async () => { + openPanel(); + const first = await editAndSave('Guard', "record.status == 'open'"); + expect(first).toEqual({ name: 'guard_hook', ifMatch: undefined, ifNoneMatch: '*' }); + }); + + it('a hook with a pending draft: the first save sends If-Match with the version its draft read served', async () => { + door.drafts.set('guard_hook', { body: { ...guard, label: 'Guard' }, version: 'hmac-sha256:pending' }); + openPanel(); + const first = await editAndSave('Guard', "record.status == 'open'"); + expect(first).toEqual({ name: 'guard_hook', ifMatch: 'hmac-sha256:pending', ifNoneMatch: undefined }); + // Read at the package the save writes. + expect(mockClient.getDraft).toHaveBeenCalledWith('hook', 'guard_hook', { packageId: PKG }); + }); + + it('each hook keeps its own pin: a hook saved, left and re-opened is pinned to its last receipt', async () => { + openPanel(); + const g1 = await editAndSave('Guard', "record.status == 'open'"); + const a1 = await editAndSave('Audit', "record.status == 'closed'"); + const g2 = await editAndSave('Guard', "record.status == 'void'"); + expect([g1, a1, g2]).toEqual([ + { name: 'guard_hook', ifMatch: undefined, ifNoneMatch: '*' }, + { name: 'audit_hook', ifMatch: undefined, ifNoneMatch: '*' }, + { name: 'guard_hook', ifMatch: 'hmac-sha256:v1', ifNoneMatch: undefined }, + ]); + }); + + it('control: an unreadable drafts ledger pins nothing, and the first save goes unpinned as before', async () => { + door.ledgerFails = true; + openPanel(); + const first = await editAndSave('Guard', "record.status == 'open'"); + expect(first).toEqual({ name: 'guard_hook', ifMatch: undefined, ifNoneMatch: undefined }); + }); + + it('a new hook is created with the first-write pin', async () => { + openPanel(); + await screen.findByText('Guard'); + fireEvent.click(screen.getByRole('button', { name: /New/ })); + await waitFor(() => expect(door.sent).toHaveLength(1)); + expect(door.sent[0]).toMatchObject({ ifMatch: undefined, ifNoneMatch: '*' }); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx index bcc852a7cb..2d4e627160 100644 --- a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx @@ -38,7 +38,7 @@ import { HookTargetScopeContext, type HookTargetScope } from '../metadata-admin/ import { loadPackageSurfaces } from './packageSurfaces.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 { readDraftForSave, useDraftSaveGuard, type DraftPin } from '../metadata-admin/DraftConflictDialog.js'; // objectui#11787 — a hook autosaves through the pillars' own autosave. import { useDraftAutoSave, type DraftSend } from './useDraftAutoSave.js'; import { t, tFormat, useMetadataLocale } from '../metadata-admin/i18n.js'; @@ -130,14 +130,15 @@ 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. A save writes the list in place (objectui#11787) - // and the re-read that follows "+ New" reads back what was written, so the - // version survives both; a conflict's "reload" re-reads the list, and a - // publish forgets the version. + // objectui#11773 — each hook's pin: what the list load read for it (its + // draft's version, or "no draft row"), then each save's receipt. A save + // writes the list in place (objectui#11787) and the re-read that follows + // "+ New" reads back what was written, so the pin survives both; a + // conflict's "reload" re-reads the list, and a publish forgets every pin. const reloadHooks = React.useCallback(() => setNonce((n) => n + 1), []); const { save: saveHookDraft, + recordRead: recordHookRead, forget: forgetHookVersion, dialog: hookConflictDialog, } = useDraftSaveGuard(client, reloadHooks); @@ -202,6 +203,13 @@ export function ObjectHooksPanel({ ); const blockingIssues = celErrors.hook === selectedHookName ? celErrors.count : 0; + // The hook the buffer holds and whether it holds an unsent edit, read by the + // list load (objectui#11773) and the install below (objectui#11787). + const bufferRef = React.useRef({ hook: draftFor, dirty }); + React.useLayoutEffect(() => { + bufferRef.current = { hook: draftFor, dirty }; + }); + React.useEffect(() => { let cancelled = false; setLoading(true); @@ -214,22 +222,37 @@ export function ObjectHooksPanel({ // Drafts win over the published row of the same name. const [pub, draftHeaders] = await Promise.all([ client.list('hook', { packageId }) as Promise, - client.listDrafts({ packageId, type: 'hook' }).catch(() => [] as Array<{ name?: string }>), + client.listDrafts({ packageId, type: 'hook' }).catch(() => null), ]); const draftNames = (draftHeaders || []).map((d) => String(d.name ?? '')).filter(Boolean); - const bodies = await Promise.all( - draftNames.map((n) => client.getDraft('hook', n).then(draftBody).catch(() => null)), - ); + // objectui#11773 — each draft is read at the address its save writes + // (`?package=` this package), so the version it serves pins that save. + const reads = await Promise.all(draftNames.map((n) => readDraftForSave(client, 'hook', n, packageId))); if (cancelled) return; const byName = new Map(); + // What the next save of each listed hook is pinned to. A hook this + // package's drafts ledger does not list has no draft at the save's + // address — when the ledger was read at all: an unreadable ledger says + // nothing, and pins nothing. + const pins = new Map(); for (const h of pub || []) { - if (h?.name && targetsObject(h, objectName)) byName.set(String(h.name), h); + if (h?.name && targetsObject(h, objectName)) { + byName.set(String(h.name), h); + pins.set(String(h.name), draftHeaders ? { ifNoneMatch: '*' } : null); + } } draftNames.forEach((n, i) => { - const body = bodies[i]; + const body = draftBody(reads[i]!.answer); if (body && targetsObject(body, objectName)) byName.set(n, body); + if (byName.has(n)) pins.set(n, reads[i]!.pin); }); setHooks([...byName.values()]); + // A hook whose buffer holds an unsent edit keeps the pin that edit was + // built on: the install below does not replace that buffer either. + const open = bufferRef.current; + for (const [n, pin] of pins) { + if (!(open.dirty && open.hook === n)) recordHookRead('hook', n, packageId, pin); + } } catch (e) { if (!cancelled) setError(formatMetadataError(e)); } finally { @@ -239,16 +262,12 @@ export function ObjectHooksPanel({ return () => { cancelled = true; }; - }, [client, packageId, objectName, nonce]); + }, [client, packageId, objectName, nonce, recordHookRead]); // Load the selected hook into an editable draft: on a selection change, and // when the list changes. objectui#11787 — except over an unsent edit of that // same hook: the list an autosave writes holds what that save sent, and an - // edit the author took meanwhile is newer than it. - const bufferRef = React.useRef({ hook: draftFor, dirty }); - React.useLayoutEffect(() => { - bufferRef.current = { hook: draftFor, dirty }; - }); + // edit the author took meanwhile is newer than it (`bufferRef`, above). React.useEffect(() => { if (selected !== null && bufferRef.current.hook === selected && bufferRef.current.dirty) return; const hook = hooks.find((h) => h.name === selected) ?? null; @@ -333,7 +352,9 @@ export function ObjectHooksPanel({ setSaving(true); setError(null); try { - await client.save('hook', name, fresh, { mode: 'draft', packageId }); + // objectui#11773 — a create is the first-write pin: refused where the + // name already holds a draft, instead of written over it. + await client.save('hook', name, fresh, { mode: 'draft', packageId, ifNoneMatch: '*' }); // objectui#11787 — a new hook is a new draft: the count says so. onDraftSaved?.(); setNonce((n) => n + 1); diff --git a/packages/app-shell/src/views/studio-design/ObjectListViewInspector.tsx b/packages/app-shell/src/views/studio-design/ObjectListViewInspector.tsx index 4215fa7183..54f7644b95 100644 --- a/packages/app-shell/src/views/studio-design/ObjectListViewInspector.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectListViewInspector.tsx @@ -64,7 +64,7 @@ import { useObjectFields } from '../metadata-admin/previews/useObjectFields.js'; import { foldStoredListOptions } from '../metadata-admin/previews/ViewPreview.js'; import { WIDGETS, type ObjectFieldOption } from '../metadata-admin/widgets.js'; import { failed, loaded, type LoadState } from '../metadata-admin/loadState.js'; -import { useDraftSaveGuard } from '../metadata-admin/DraftConflictDialog.js'; +import { readDraftForSave, useDraftSaveGuard } from '../metadata-admin/DraftConflictDialog.js'; import { t, tFormat } from '../metadata-admin/i18n.js'; import { defaultListColumnsFromObject } from '../ObjectView.js'; import { findByName } from '../../hooks/useNavTargetLabel.js'; @@ -276,7 +276,7 @@ export function useObjectListViewDraft({ const [reloadNonce, setReloadNonce] = React.useState(0); const reload = React.useCallback(() => setReloadNonce((n) => n + 1), []); // objectui#11773 — one guard per editing buffer; a conflict's "reload" re-runs this load. - const { save: guardedSave, forget, dialog: conflictDialog } = useDraftSaveGuard(client, reload); + const { save: guardedSave, recordRead, dialog: conflictDialog } = useDraftSaveGuard(client, reload); React.useEffect(() => { setFailure(null); @@ -287,8 +287,11 @@ export function useObjectListViewDraft({ let cancelled = false; (async () => { try { - const draftResp = await client.getDraft('view', viewId).catch(() => null); + // objectui#11773 — read at the address the save below writes, so the + // version it serves pins that save. + const draftRead = await readDraftForSave(client, 'view', viewId, packageId); if (cancelled) return; + const draftResp = draftRead.answer; // A served draft is the whole document, taken as-is (objectui#10765); // the published layer only without one. Both lose the read // decorations a save must not send back. @@ -300,8 +303,8 @@ export function useObjectListViewDraft({ if (cancelled) return; const published = (lay as { effective?: unknown; code?: unknown } | null)?.effective ?? (lay as { code?: unknown } | null)?.code; const row = draft ?? (isPlainObject(published) ? (stripReadDecorations(published) as Row) : null); - // objectui#11773 — a read serves no version: the next save is unpinned. - forget(); + // objectui#11773 — the next save is pinned to what the draft read served. + recordRead('view', viewId, packageId, draftRead.pin); setBuffer({ for: listViewBufferKey(viewId), row, dirty: false, hasDraft: !!draft }); } catch (e) { if (!cancelled) setFailure({ during: 'load', error: e }); @@ -310,7 +313,7 @@ export function useObjectListViewDraft({ return () => { cancelled = true; }; - }, [client, viewId, publishNonce, reloadNonce, forget]); + }, [client, viewId, packageId, publishNonce, reloadNonce, recordRead]); const edit = React.useCallback((row: Row) => { setBuffer((b) => ({ ...b, row, dirty: true })); 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 index 1a13b1fd60..d26248dfe9 100644 --- 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 @@ -8,17 +8,19 @@ * 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. + * The server double answers the way the `/meta` draft door of the published + * `@objectstack/*` 18.0.0-next.0 was measured to (readings on the pull + * request): a draft save returns its `version`; the draft READ serves the + * draft's `version` too, and no draft at all is `null` from `getDraft`; an + * `If-Match` that is not the current draft's version, or `If-None-Match: *` + * over an existing draft, is `409 METADATA_CONFLICT` carrying + * `currentVersion`; a save with neither is last-writer-wins. 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 first save after a load is pinned to that load's read: `If-Match` with + * the draft's version, or `If-None-Match: *` when no draft was pending — so the + * card's literal race (two editors who each load and save ONCE) is refused too. * * 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. @@ -49,6 +51,8 @@ interface SaveCall { name: string; body: Record; ifMatch: string | undefined; + ifNoneMatch: string | undefined; + packageId: string | undefined; } const server = vi.hoisted(() => ({ @@ -70,6 +74,7 @@ async function conflictRefusal(type: string, name: string, head: string | null): 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', + currentVersion: head, }), { status: 409, headers: { 'content-type': 'application/json' } }, )) as unknown as typeof fetch, @@ -98,18 +103,21 @@ const mockClient = vi.hoisted(() => { 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) => { + // The draft read serves the body and its version; no draft is `null`, as + // the real client resolves the read's 404. + getDraft: vi.fn(async (type: string, name: string, _options?: { packageId?: 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 }); + if (!draft) return null; + return { type, name, item: JSON.parse(JSON.stringify(draft.body)) as Record, version: draft.version }; }), - save: vi.fn(async (type: string, name: string, item: unknown, options?: { ifMatch?: string }) => { + save: vi.fn(async (type: string, name: string, item: unknown, options?: { ifMatch?: string; ifNoneMatch?: string; packageId?: string }) => { const body = JSON.parse(JSON.stringify(item)) as Record; const ifMatch = options?.ifMatch; - server.saves.push({ type, name, body, ifMatch }); + const ifNoneMatch = options?.ifNoneMatch; + server.saves.push({ type, name, body, ifMatch, ifNoneMatch, packageId: options?.packageId }); const head = server.drafts.get(k(type, name))?.version ?? null; if (ifMatch !== undefined && ifMatch !== head) throw await conflictRefusal(type, name, head); + if (ifNoneMatch !== undefined && head !== null) throw await conflictRefusal(type, name, head); server.seq += 1; const version = `hmac-sha256:v${server.seq}`; server.drafts.set(k(type, name), { body, version }); @@ -224,16 +232,46 @@ async function raceTwoEditors(): Promise<{ a: HTMLElement; b: HTMLElement }> { } 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 () => { + it('one editor: the first autosave is pinned to the load ("no draft"), and each later one to the receipt before it', 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(server.saves.map((s) => [s.ifMatch, s.ifNoneMatch])).toEqual([ + [undefined, '*'], + ['hmac-sha256:v1', undefined], + ['hmac-sha256:v2', undefined], + ]); + // The draft was read at the package the saves write. + expect(mockClient.getDraft).toHaveBeenCalledWith('object', OBJECT.name, { packageId: PKG }); + expect(server.saves.every((s) => s.packageId === PKG)).toBe(true); expect(screen.queryByTestId('draft-conflict-dialog')).toBeNull(); }); + it('the card\'s race: two editors who each load and save ONCE — the later first save is refused, and the earlier one\'s field survives', async () => { + const a = openEditor(); + await loaded(a, '2 fields'); + const b = openEditor(); + await loaded(b, '2 fields'); + await addFieldAndSave(a); // A: + field_3, the first draft + expect(serverDraftFields()).toEqual(['title', 'status', 'field_3']); + fireEvent.click(within(b).getByTitle(/^Add a field/)); // B's first save, built on the published object + await screen.findByTestId('draft-conflict-dialog', undefined, { timeout: 8000 }); + const refused = server.saves[server.saves.length - 1]!; + expect([refused.ifMatch, refused.ifNoneMatch]).toEqual([undefined, '*']); + // A's field is still in the draft: B's stale copy was not written over it. + expect(serverDraftFields()).toEqual(['title', 'status', 'field_3']); + }); + + it('an editor that opens on a pending draft pins its first save to the version that read served', async () => { + server.drafts.set(`object/${OBJECT.name}`, { body: JSON.parse(JSON.stringify(OBJECT)), version: 'hmac-sha256:pending' }); + const a = openEditor(); + await loaded(a, '2 fields'); + await addFieldAndSave(a); + expect(server.saves.map((s) => [s.ifMatch, s.ifNoneMatch])).toEqual([['hmac-sha256:pending', undefined]]); + }); + 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]!; @@ -251,7 +289,7 @@ describe('Data pillar — a draft saved elsewhere is not overwritten in silence expect(serverDraftFields()).toEqual(['status', 'title', 'field_3']); }); - it('"reload" replaces the stale buffer with the saved draft, and the next save is unpinned', async () => { + it('"reload" replaces the stale buffer with the saved draft, and the next save is pinned to that draft\'s version', async () => { const { a } = await raceTwoEditors(); const savesBefore = server.saves.length; fireEvent.click(screen.getByTestId('draft-conflict-reload')); @@ -261,12 +299,12 @@ describe('Data pillar — a draft saved elsewhere is not overwritten in silence await addFieldAndSave(a); const next = server.saves[server.saves.length - 1]!; - expect(next.ifMatch).toBeUndefined(); + expect(next.ifMatch).toBe('hmac-sha256:v2'); // 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 () => { + it('"overwrite" asks again, then re-sends pinned to the refusal\'s currentVersion 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. @@ -278,7 +316,8 @@ describe('Data pillar — a draft saved elsewhere is not overwritten in silence }); await waitFor(() => expect(server.saves.length).toBe(savesBefore + 1), { timeout: 8000 }); const overwrite = server.saves[server.saves.length - 1]!; - expect(overwrite.ifMatch).toBeUndefined(); + // B's draft is the version the refusal named. + expect([overwrite.ifMatch, overwrite.ifNoneMatch]).toEqual(['hmac-sha256:v2', undefined]); expect(serverDraftFields()).toEqual(['title', 'status', 'field_3', 'field_4']); await waitFor(() => expect(within(a).queryByTestId('data-autosaving')).toBeNull(), { timeout: 8000 }); @@ -287,8 +326,8 @@ describe('Data pillar — a draft saved elsewhere is not overwritten in silence }); }); -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 () => { +describe('Data pillar — a create sends If-None-Match: * (objectui#11773)', () => { + it('the new object\'s skeleton is sent with the first-write pin; its first edit after the load is pinned to the read', async () => { server.active.clear(); const a = openEditor(); fireEvent.click(await within(a).findByTestId('empty-state-new-object', undefined, { timeout: 8000 })); @@ -296,11 +335,15 @@ describe('Data pillar — a create sends no If-Match (objectui#11773)', () => { 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(); + expect([server.saves[0]!.ifMatch, server.saves[0]!.ifNoneMatch]).toEqual([undefined, '*']); await loaded(a, '1 fields'); await addFieldAndSave(a); await addFieldAndSave(a); - expect(server.saves.map((s) => s.ifMatch)).toEqual([undefined, undefined, 'hmac-sha256:v2']); + expect(server.saves.map((s) => [s.ifMatch, s.ifNoneMatch])).toEqual([ + [undefined, '*'], + ['hmac-sha256:v1', undefined], + ['hmac-sha256:v2', undefined], + ]); }); }); diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index 6579500596..188311f942 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -140,7 +140,7 @@ import { useRegisteredMetadataDefaultInspector } from '../metadata-admin/default 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 { readDraftForSave, useDraftSaveGuard } from '../metadata-admin/DraftConflictDialog.js'; import { DESIGNER_SURFACE_PARAM, formatSurfaceParam, @@ -1368,11 +1368,13 @@ export function StudioDesignSurface({ aiSlot }: StudioDesignSurfaceProps): React setAppErr(null); try { const navObjects = appAddObjects ? await loadPackageObjects().catch(() => []) : []; + // objectui#11773 — a create is the first-write pin: refused where the + // name already holds a draft, instead of written over it. await shellClient.save( 'app', name, buildAppSkeleton(name, label, navObjects), - { mode: 'draft', packageId }, + { mode: 'draft', packageId, ifNoneMatch: '*' }, ); toast.success(tFormat('engine.studio.app.savedDraft', locale, { label })); setAppDraftPending(label); @@ -2800,6 +2802,7 @@ export function InterfacesPillar({ const reloadLeafDraft = React.useCallback(() => setLeafReloadNonce((n) => n + 1), []); const { save: saveLeafDraft, + recordRead: recordLeafRead, forget: forgetLeafVersion, dialog: leafConflictDialog, } = useDraftSaveGuard(client, reloadLeafDraft); @@ -2812,7 +2815,7 @@ export function InterfacesPillar({ }, []); const { save: saveNavDraft, - forget: forgetNavVersion, + recordRead: recordNavRead, dialog: navConflictDialog, } = useDraftSaveGuard(client, reloadNavDraft); // The app load also re-reads after every draft save in the package (the @@ -3031,11 +3034,14 @@ export function InterfacesPillar({ // in the drafts ledger: it has never been saved, so it has no layer to // read and `/layers` would answer 404. Its draft is the whole document. const saved = !!published?.[0]?.name; - const [layRaw, appDraftResp] = await Promise.all([ + // objectui#11773 — the draft is read at the address the nav save writes + // (`?package=` this package), so the version it serves pins that save. + const [layRaw, appDraftRead] = await Promise.all([ saved ? client.layered>('app', name) : null, - client.getDraft>('app', name).catch(() => null), + readDraftForSave>(client, 'app', name, packageId), ]); if (cancelled) return; + const appDraftResp = appDraftRead.answer; const lay = layRaw as { effective?: Record; code?: Record } | null; const eff = (lay?.effective ?? lay?.code ?? {}) as Record; const appDraftBody = extractDraftBody(appDraftResp); @@ -3056,9 +3062,12 @@ 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(); + // objectui#11773 — the next nav save is pinned to what this read + // served, unless it is the read-back of this pillar's own nav save + // (see `navEchoRef`), whose receipt already pins it. + if (!isSameApp || navEchoRef.current !== publishNonce) { + recordNavRead('app', name, packageId, appDraftRead.pin); + } navEchoRef.current = null; } navBaselineRef.current = body; @@ -3099,7 +3108,7 @@ export function InterfacesPillar({ return () => { cancelled = true; }; - }, [client, packageId, publishNonce, draftNonce, metadataRefreshNonce, navReloadNonce, forgetNavVersion]); + }, [client, packageId, publishNonce, draftNonce, metadataRefreshNonce, navReloadNonce, recordNavRead]); const Preview = useRegisteredMetadataPreview(current?.type ?? ''); // Studio-canvas surface override: the SAME type can render as a different @@ -3209,11 +3218,11 @@ export function InterfacesPillar({ setSelection(null); (async () => { try { - const draftResp = await client - .getDraft>(current.type, current.name) - .catch(() => null); + // objectui#11773 — read at the address the leaf save writes, so the + // version it serves pins that save. + const draftRead = await readDraftForSave>(client, current.type, current.name, packageId); if (cancelled) return; - const body = extractDraftBody(draftResp); + const body = extractDraftBody(draftRead.answer); // objectui#11799 — the baseline is read only when it is used, for a // leaf with no pending draft. An unsaved leaf always has one, so it // never asks `/layers`, which answers 404 for a name with no layer. @@ -3226,8 +3235,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(); + // objectui#11773 — the next save is pinned to what the draft read served. + recordLeafRead(current.type, current.name, packageId, draftRead.pin); setHasDraft(!!body); setIfDirty(false); } catch (e) { @@ -3249,7 +3258,7 @@ export function InterfacesPillar({ // same rule. if (!settled) setLoading(false); }; - }, [client, current, isEditable, publishNonce, leafReloadNonce, forgetLeafVersion]); + }, [client, current, isEditable, packageId, publishNonce, leafReloadNonce, recordLeafRead, forgetLeafVersion]); // objectui#5813 — a local dirty flag so auto-save only arms after a real // edit, never on the load-effect's own setDraft. @@ -3477,7 +3486,9 @@ export function InterfacesPillar({ : kind === 'report' ? buildReportSkeleton(name, label, binding) : buildPageSkeleton(name, label, sourceKind, t('engine.studio.interfaces.create.pageStarter', locale)); - await client.save(kind, name, body, { mode: 'draft', packageId }); + // objectui#11773 — and the door refuses it too (the first-write pin), + // for a draft that lands between the check above and this save. + await client.save(kind, name, body, { mode: 'draft', packageId, ifNoneMatch: '*' }); onDraftSaved?.(); // The entry that links it, appended to the app document and sent // through the nav editor's own save, as an edit there would be: its @@ -4568,7 +4579,7 @@ export function DataPillar({ const reloadObjDraft = React.useCallback(() => setObjReloadNonce((n) => n + 1), []); const { save: saveObjDraft, - forget: forgetObjVersion, + recordRead: recordObjRead, dialog: objConflictDialog, } = useDraftSaveGuard(client, reloadObjDraft); // Left-rail search + inline "new object" creator (design §4: rail = search + New). @@ -4712,19 +4723,21 @@ export function DataPillar({ setDirty(false); (async () => { try { - const [layRaw, draftResp] = await Promise.all([ + // objectui#11773 — the draft is read at the address the object saves + // write, so the version it serves pins the first of them. + const [layRaw, draftRead] = await Promise.all([ client.layered>('object', current.name), - client.getDraft>('object', current.name).catch(() => null), + readDraftForSave>(client, 'object', current.name, packageId), ]); if (cancelled) return; const lay = layRaw as { effective?: Record; code?: Record }; const baseline = (lay.effective ?? lay.code ?? {}) as Record; - const draftBody = extractDraftBody(draftResp); + const draftBody = extractDraftBody(draftRead.answer); // Served draft as-is, baseline only without one (objectui#10765). setObjDraft(draftBody ?? baseline); setObjDraftFor(`object:${current.name}`); - // objectui#11773 — a read serves no version: the next save is unpinned. - forgetObjVersion(); + // objectui#11773 — the next save is pinned to what the draft read served. + recordObjRead('object', current.name, packageId, draftRead.pin); // objectui#11272 — the buffer installed is clean, as every pillar's // is: an edit a period began while it was another object's is dropped // with it, never sent as this object's. @@ -4757,7 +4770,7 @@ export function DataPillar({ loadedNameRef.current = null; } }; - }, [client, current, publishNonce, objReloadNonce, forgetObjVersion]); + }, [client, current, packageId, publishNonce, objReloadNonce, recordObjRead]); const fieldCount = React.useMemo(() => readFields(objDraft.fields).entries.length, [objDraft]); @@ -4878,7 +4891,9 @@ export function DataPillar({ setError(null); try { const body = buildObjectSkeleton(name, label, t('engine.studio.data.nameFieldLabel', locale), sharingModel); - await client.save('object', name, body, { mode: 'draft', packageId }); + // objectui#11773 — a create is the first-write pin: refused where the + // name already holds a draft, instead of written over it. + await client.save('object', name, body, { mode: 'draft', packageId, ifNoneMatch: '*' }); const surface: Surface = { type: 'object', name, label }; setObjects((prev) => [...prev, surface]); setCurrent(surface); @@ -5918,7 +5933,7 @@ export function AutomationsPillar({ const reloadFlowDraft = React.useCallback(() => setFlowReloadNonce((n) => n + 1), []); const { save: saveFlowDraft, - forget: forgetFlowVersion, + recordRead: recordFlowRead, dialog: flowConflictDialog, } = useDraftSaveGuard(client, reloadFlowDraft); const [selection, setSelection] = React.useState(null); @@ -6082,7 +6097,9 @@ export function AutomationsPillar({ }, trigger, ); - await client.save('flow', name, skeleton, { mode: 'draft', packageId: draftPackageId }); + // objectui#11773 — a create is the first-write pin: refused where the + // name already holds a draft, instead of written over it. + await client.save('flow', name, skeleton, { mode: 'draft', packageId: draftPackageId, ifNoneMatch: '*' }); const item: Surface = { type: 'flow', name, label }; setFlows((fs) => [...fs.filter((f) => f.name !== name), item]); setCurrent(item); @@ -6110,11 +6127,11 @@ export function AutomationsPillar({ setSelection(null); (async () => { try { - const draftResp = await client - .getDraft>('flow', current.name) - .catch(() => null); + // objectui#11773 — read at the address the flow saves write, so the + // version it serves pins the first of them. + const draftRead = await readDraftForSave>(client, 'flow', current.name, draftPackageId); if (cancelled) return; - const draftBody = extractDraftBody(draftResp); + const draftBody = extractDraftBody(draftRead.answer); // objectui#11799 — the baseline is read only for a flow with no // pending draft, the one case it is used. An unsaved flow always has // one, so it never asks `/layers`, which answers 404 for it. @@ -6125,8 +6142,8 @@ export function AutomationsPillar({ // Served draft as-is, baseline only without one (objectui#10765). setDraft(draftBody ?? baseline); setDraftFor(`flow:${current.name}`); - // objectui#11773 — a read serves no version: the next save is unpinned. - forgetFlowVersion(); + // objectui#11773 — the next save is pinned to what the draft read served. + recordFlowRead('flow', current.name, draftPackageId, draftRead.pin); setHasDraft(!!draftBody); } catch (e) { if (!cancelled) setError(plainRefusal(e)); @@ -6144,7 +6161,7 @@ export function AutomationsPillar({ // `loading` it raised, as in InterfacesPillar's draft load. if (!settled) setLoading(false); }; - }, [client, current, publishNonce, flowReloadNonce, forgetFlowVersion]); + }, [client, current, draftPackageId, publishNonce, flowReloadNonce, recordFlowRead]); // objectui#5813 — local dirty flag: auto-save arms only after a real edit. const [autoDirty, setAutoDirty] = React.useState(false); @@ -6906,7 +6923,13 @@ export function AccessPillar({ try { // Package door → create as a DRAFT stamped with this package (D6/D7), // published atomically with the rest of the package. - await client.save('permission', name, buildPermissionSkeleton(name, label), { mode: 'draft', packageId }); + // objectui#11773 — a create is the first-write pin: refused where the + // name already holds a draft, instead of written over it. + await client.save('permission', name, buildPermissionSkeleton(name, label), { + mode: 'draft', + packageId, + ifNoneMatch: '*', + }); toast.success(tFormat('engine.studio.access.created', locale, { label })); setCreating(false); onDraftSaved?.(); diff --git a/packages/data-objectstack/src/metadata-client.firstWritePin-11773.test.ts b/packages/data-objectstack/src/metadata-client.firstWritePin-11773.test.ts new file mode 100644 index 0000000000..aecd04788e --- /dev/null +++ b/packages/data-objectstack/src/metadata-client.firstWritePin-11773.test.ts @@ -0,0 +1,99 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — the save options' two preconditions on the `/meta` PUT door + * (ADR-0008): `ifMatch` → `If-Match: `, `ifNoneMatch` → `If-None-Match: *`. + * + * The door refuses the pair `400` (`If-Match` needs a current row, `*` needs + * none, so they can never both hold), measured on the published + * `@objectstack/*` 18.0.0-next.0 server. The client refuses it first, before + * any request, so a confused caller learns it at the call and not as a 400. + * + * The read half: `getDraft` hands the caller the envelope the draft read + * served, `version` included — the token a draft save at the same address + * pins with (`GetMetaItemResponseSchema`'s `version`). + */ + +import { describe, it, expect, vi } from 'vitest'; +import { MetadataClient } from './metadata-client'; + +interface Sent { + url: string; + method: string; + headers: Headers; +} + +function clientOver(answer: (url: string, init: RequestInit) => Response) { + const sent: Sent[] = []; + const fetchImpl = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + sent.push({ url, method: init?.method ?? 'GET', headers: new Headers(init?.headers) }); + return answer(url, init ?? {}); + }) as unknown as typeof fetch; + return { client: new MetadataClient({ baseUrl: 'http://localhost:3000', fetch: fetchImpl }), sent }; +} + +const json = (status: number, body: unknown) => + new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); + +const receipt = { success: true, version: 'hmac-sha256:aa', seq: 1, state: 'draft', message: 'Saved' }; + +describe('MetadataClient.save — the first-write pin (objectui#11773)', () => { + it('sends ifNoneMatch as `If-None-Match: *`, and no If-Match', async () => { + const { client, sent } = clientOver(() => json(200, receipt)); + await client.save('object', 'acme_note', { name: 'acme_note' }, { mode: 'draft', packageId: 'com.acme.app', ifNoneMatch: '*' }); + expect(sent).toHaveLength(1); + expect(sent[0]!.method).toBe('PUT'); + expect(sent[0]!.headers.get('If-None-Match')).toBe('*'); + expect(sent[0]!.headers.get('If-Match')).toBeNull(); + }); + + it('control: ifMatch alone is sent as If-Match, with no If-None-Match', async () => { + const { client, sent } = clientOver(() => json(200, receipt)); + await client.save('object', 'acme_note', { name: 'acme_note' }, { mode: 'draft', ifMatch: 'hmac-sha256:aa' }); + expect(sent[0]!.headers.get('If-Match')).toBe('hmac-sha256:aa'); + expect(sent[0]!.headers.get('If-None-Match')).toBeNull(); + }); + + it('control: neither option sends neither header (unpinned, as before)', async () => { + const { client, sent } = clientOver(() => json(200, receipt)); + await client.save('object', 'acme_note', { name: 'acme_note' }, { mode: 'draft' }); + expect(sent[0]!.headers.get('If-Match')).toBeNull(); + expect(sent[0]!.headers.get('If-None-Match')).toBeNull(); + }); + + it('never sends the two together: both set is refused before any request', async () => { + const { client, sent } = clientOver(() => json(200, receipt)); + await expect( + client.save('object', 'acme_note', { name: 'acme_note' }, { ifMatch: 'hmac-sha256:aa', ifNoneMatch: '*' }), + ).rejects.toThrow(/mutually exclusive/); + expect(sent).toEqual([]); + }); + + it('the door\'s refusal of a pinned save reaches the caller parsed, with the body kept for its currentVersion', async () => { + const refusal = { + error: 'object/acme_note has been modified since you loaded it. Expected parent null but current is hmac-sha256:bb.', + code: 'METADATA_CONFLICT', + currentVersion: 'hmac-sha256:bb', + }; + const { client } = clientOver(() => json(409, refusal)); + const err = await client + .save('object', 'acme_note', { name: 'acme_note' }, { mode: 'draft', ifNoneMatch: '*' }) + .catch((e: unknown) => e); + expect(err).toMatchObject({ status: 409, code: 'METADATA_CONFLICT', body: refusal }); + }); +}); + +describe('MetadataClient.getDraft — the read surfaces its version (objectui#11773)', () => { + it('hands back the served envelope with `version`, read at the package it names', async () => { + const { client, sent } = clientOver((url) => { + if (url.endsWith('/meta/_drafts')) return json(200, { drafts: [{ type: 'object', name: 'acme_note' }] }); + return json(200, { type: 'object', name: 'acme_note', item: { name: 'acme_note' }, version: 'hmac-sha256:aa' }); + }); + const envelope = await client.getDraft<{ version?: unknown }>('object', 'acme_note', { packageId: 'com.acme.app' }); + expect(envelope?.version).toBe('hmac-sha256:aa'); + const read = sent.find((s) => s.url.includes('/meta/object/acme_note'))!; + expect(new URL(read.url).searchParams.get('package')).toBe('com.acme.app'); + expect(new URL(read.url).searchParams.get('state')).toBe('draft'); + }); +}); diff --git a/packages/data-objectstack/src/metadata-client.ts b/packages/data-objectstack/src/metadata-client.ts index 0aa982825f..cdd84a74a3 100644 --- a/packages/data-objectstack/src/metadata-client.ts +++ b/packages/data-objectstack/src/metadata-client.ts @@ -292,17 +292,41 @@ export type MetadataPublishPackageDraftsResult = Partial Date: Wed, 14 Oct 2026 03:32:25 +0000 Subject: [PATCH 2/5] test(app-shell): pin the first save of the permission matrix and the list-view panel, and move create/first-save expectations to the pins (objectui#11773) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- ...atrixEditor.firstSavePinned-11773.test.tsx | 123 +++++++++++ ...ooksPanel.designerRegistryMissing.test.tsx | 4 +- ...ewInspector.firstSavePinned-11773.test.tsx | 192 ++++++++++++++++++ ...ce.createAppOpensInterfaces-11794.test.tsx | 13 +- ...ignSurface.interfacesCreate-11823.test.tsx | 10 +- ...urface.interfacesCreatePage-11823.test.tsx | 7 +- ...gnSurface.listViewInspector-11823.test.tsx | 11 +- ...oDesignSurface.sharedSelect-11865.test.tsx | 11 +- 8 files changed, 353 insertions(+), 18 deletions(-) create mode 100644 packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.firstSavePinned-11773.test.tsx create mode 100644 packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx diff --git a/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.firstSavePinned-11773.test.tsx b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.firstSavePinned-11773.test.tsx new file mode 100644 index 0000000000..fdec25fcac --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.firstSavePinned-11773.test.tsx @@ -0,0 +1,123 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — the permission matrix's PACKAGE door pins its first draft + * save to what its load read. + * + * The load reads the set's draft at the package the save writes + * (`getDraft(type, name, { packageId })`), and its answer pins the first + * autosave: `If-Match` with the version a pending draft's read served, or + * `If-None-Match: *` when no draft was pending. The ENVIRONMENT door writes live + * config, reads no draft and pins nothing, as before (the control). + * + * The client double answers as the published `@objectstack/*` 18.0.0-next.0 + * door was measured to (readings on the pull request): the draft read serves + * `version`, and no draft at all is `null`. + */ + +import '@testing-library/jest-dom/vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +const PUBLISHED = { + name: 'sales_perms', + label: 'Sales', + objects: { a_account: { allowRead: true, allowCreate: true } }, + fields: {}, +}; + +const LAYERED = { effective: PUBLISHED, code: null, overlay: null, overlayScope: null }; + +interface Server { + draft: { body: Record; version: string } | null; + draftReads: unknown[][]; + savedOpts: Array | undefined>; +} + +type FakeClient = Record Promise>; + +function makeClient(server: Server): FakeClient { + return { + layered: async () => LAYERED, + getDraft: async (...args: unknown[]) => { + server.draftReads.push(args); + return server.draft ? { type: 'permission', name: 'sales_perms', item: server.draft.body, version: server.draft.version } : null; + }, + list: async (type: string) => (type === 'object' ? [{ item: { name: 'a_account' } }] : type === 'permission' ? [PUBLISHED] : []), + get: async () => null, + save: async (_type: string, _name: string, payload: Record, saveOpts?: Record) => { + server.savedOpts.push(saveOpts); + return { success: true, version: 'hmac-sha256:receipt', state: 'draft', payload }; + }, + }; +} + +let clientImpl: FakeClient; + +vi.mock('./useMetadata', () => ({ + useMetadataClient: () => clientImpl, + useMetadataTypes: () => ({ + loading: false, + error: null, + entries: [{ type: 'permission', label: 'Permission', allowEnvironmentOverlay: true }], + }), +})); + +vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null })); + +import { PermissionMatrixEditPage } from './PermissionMatrixEditor'; + +afterEach(cleanup); + +const AUTOSAVE = { timeout: 4000 }; + +function renderDoor(packageId: string | undefined) { + return render( + + + , + ); +} + +const accountRow = () => screen.getByText('a_account').closest('tr')!; + +describe('the package door pins its first draft save to its load (objectui#11773)', () => { + it('no pending draft: the first autosave sends If-None-Match: *, read at the package the save writes', async () => { + const server: Server = { draft: null, draftReads: [], savedOpts: [] }; + clientImpl = makeClient(server); + renderDoor('app.a'); + await screen.findByText('a_account'); + + fireEvent.click(within(accountRow()).getByRole('button', { name: 'None' })); + await waitFor(() => expect(server.savedOpts).toHaveLength(1), AUTOSAVE); + expect(server.savedOpts[0]).toMatchObject({ mode: 'draft', packageId: 'app.a', ifNoneMatch: '*' }); + expect(server.savedOpts[0]).not.toHaveProperty('ifMatch'); + expect(server.draftReads).toContainEqual(['permission', 'sales_perms', { packageId: 'app.a' }]); + }); + + it('a pending draft: the first autosave sends If-Match with the version the read served', async () => { + const server: Server = { draft: { body: PUBLISHED, version: 'hmac-sha256:pending' }, draftReads: [], savedOpts: [] }; + clientImpl = makeClient(server); + renderDoor('app.a'); + await screen.findByText('a_account'); + + fireEvent.click(within(accountRow()).getByRole('button', { name: 'None' })); + await waitFor(() => expect(server.savedOpts).toHaveLength(1), AUTOSAVE); + expect(server.savedOpts[0]).toMatchObject({ mode: 'draft', packageId: 'app.a', ifMatch: 'hmac-sha256:pending' }); + expect(server.savedOpts[0]).not.toHaveProperty('ifNoneMatch'); + }); + + it('control: the environment door reads no draft and its live Save carries no pin, as before', async () => { + const server: Server = { draft: null, draftReads: [], savedOpts: [] }; + clientImpl = makeClient(server); + renderDoor(undefined); + await screen.findByText('a_account'); + + fireEvent.click(within(accountRow()).getByRole('button', { name: 'None' })); + fireEvent.click(screen.getByRole('button', { name: /^Save$/ })); + await waitFor(() => expect(server.savedOpts).toHaveLength(1)); + expect(server.savedOpts[0]).toEqual({ force: false }); + expect(server.draftReads).toEqual([]); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.designerRegistryMissing.test.tsx b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.designerRegistryMissing.test.tsx index 6979f12624..d1b28ff7b1 100644 --- a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.designerRegistryMissing.test.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.designerRegistryMissing.test.tsx @@ -102,7 +102,9 @@ describe('ObjectHooksPanel — no curated hook editor registered (#6846)', () => 'hook', 'guard_hook', expect.objectContaining({ handler: 'guard_fn_v2' }), - { mode: 'draft', packageId: 'com.example.showcase' }, + // objectui#11773 — no draft of this hook was pending at the load: its + // first save is the first-write pin. + { mode: 'draft', packageId: 'com.example.showcase', ifNoneMatch: '*' }, ); }); }); diff --git a/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx b/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx new file mode 100644 index 0000000000..377aed3730 --- /dev/null +++ b/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx @@ -0,0 +1,192 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11773 — an `object` leaf's list-view buffer pins its first draft + * save to the draft read its load made. + * + * The load reads the view's draft at the package the save writes + * (`?state=draft&package=P`): a read naming no package serves a package-bound + * draft's body with `version: null`, which says nothing about a save to `P`. + * What that read served pins the first save: `If-Match` with a pending draft's + * version, or `If-None-Match: *` when none was pending. + * + * The REAL hook (`useObjectListViewDraft`) over a REAL `MetadataClient`, whose + * transport is an in-memory server answering as the published + * `@objectstack/*` 18.0.0-next.0 door was measured to (readings on the pull + * request). + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { MetadataCtx, type MetadataContextValue } from '@object-ui/react'; +import { MetadataClient } from '@object-ui/data-objectstack'; +import { useObjectListViewDraft, type ObjectListViewDraft } from './ObjectListViewInspector'; +import type { Surface } from './navSurface'; + +const PKG = 'com.acme.app'; +const VIEW = 'showcase_task.default'; + +const OBJECT_DEF = { + name: 'showcase_task', + label: 'Task', + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + status: { name: 'status', label: 'Status', type: 'text' }, + }, +}; + +const viewItem = (columns: string[]) => ({ + name: VIEW, + object: 'showcase_task', + viewKind: 'list', + isDefault: true, + config: { type: 'grid', data: { provider: 'object', object: 'showcase_task' }, columns }, +}); + +const server = { + active: new Map>(), + /** Draft rows by `type/name@package`. */ + drafts: new Map; version: string }>(), + draftReads: [] as string[], + puts: [] as Array<{ ifMatch: string | null; ifNoneMatch: string | null; status: number }>, + seq: 0, +}; + +const json = (status: number, body: unknown) => + new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } }); + +async function serve(input: RequestInfo | URL, init?: RequestInit): Promise { + const method = (init?.method ?? 'GET').toUpperCase(); + const url = new URL(String(input), 'http://studio.test'); + const q = url.searchParams; + const meta = url.pathname.match(/^\/api\/v1\/meta\/(.+)$/); + if (!meta) return json(200, []); + const seg = meta[1].split('/').map(decodeURIComponent); + const item = `${seg[0]}/${seg[1]}`; + if (seg[0] === '_drafts') { + return json(200, { drafts: [...server.drafts.keys()].map((k) => ({ type: k.split('/')[0], name: k.split('/')[1]!.split('@')[0] })) }); + } + if (seg.length === 3 && seg[2] === 'layers') { + const row = server.active.get(item); + return row ? json(200, { code: null, overlay: row, overlayScope: 'env', effective: row }) : json(404, { error: { code: 'NOT_FOUND' } }); + } + const address = `${item}@${q.get('package') ?? ''}`; + if (method === 'PUT') { + const headers = new Headers(init?.headers); + const ifMatch = headers.get('If-Match'); + const ifNoneMatch = headers.get('If-None-Match'); + const head = server.drafts.get(address)?.version ?? null; + const refused = (ifMatch !== null && ifMatch !== head) || (ifNoneMatch !== null && head !== null); + server.puts.push({ ifMatch, ifNoneMatch, status: refused ? 409 : 200 }); + if (refused) return json(409, { error: `${item} has been modified since you loaded it.`, code: 'METADATA_CONFLICT', currentVersion: head }); + server.seq += 1; + const version = `hmac-sha256:v${server.seq}`; + server.drafts.set(address, { body: JSON.parse(String(init?.body ?? '{}')) as Record, version }); + return json(200, { success: true, version, seq: server.seq, state: 'draft' }); + } + if (q.get('state') === 'draft') { + server.draftReads.push(url.search); + const own = server.drafts.get(address); + // Content falls back to any package's draft; the version is the address's. + const served = own ?? [...server.drafts.entries()].find(([k]) => k.startsWith(`${item}@`))?.[1]; + if (!served) return json(404, { error: 'No pending draft.', code: 'NO_DRAFT' }); + return json(200, { type: seg[0], name: seg[1], item: { ...served.body, _draft: true }, version: own ? own.version : null }); + } + const row = server.active.get(item); + return row ? json(200, { type: seg[0], name: seg[1], item: row }) : json(404, { error: { code: 'NOT_FOUND' } }); +} + +const client = new MetadataClient({ baseUrl: '', fetch: vi.fn(serve) as unknown as typeof fetch }); + +const METADATA: MetadataContextValue = { + apps: [], + objects: [OBJECT_DEF], + dashboards: [], + reports: [], + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], +}; + +const LEAF: Surface = { type: 'object', name: 'showcase_task', label: 'Tasks' }; + +const hook = { current: null as ObjectListViewDraft | null }; + +function Probe() { + hook.current = useObjectListViewDraft({ client, packageId: PKG, leaf: LEAF, publishNonce: 0 }); + return hook.current.conflictDialog; +} + +async function flush() { + await act(async () => { + for (let i = 0; i < 20; i++) await new Promise((r) => setTimeout(r, 0)); + }); +} + +/** Open the leaf, edit the view's columns, and send the save the autosave would. */ +async function openEditAndSave(columns: string[]) { + render( + + + , + ); + await flush(); + await act(async () => hook.current!.edit(viewItem(columns))); + await act(async () => hook.current!.save({ unmoved: () => true })); + await flush(); +} + +beforeEach(() => { + server.active.clear(); + server.drafts.clear(); + server.draftReads = []; + server.puts = []; + server.seq = 0; +}); +afterEach(cleanup); + +describe('useObjectListViewDraft — the first save after a load is pinned (objectui#11773)', () => { + it('a published view with no pending draft: the first save sends If-None-Match: *, the draft read named the package', async () => { + server.active.set(`view/${VIEW}`, viewItem(['title'])); + await openEditAndSave(['title', 'status']); + expect(server.puts).toEqual([{ ifMatch: null, ifNoneMatch: '*', status: 200 }]); + expect(server.draftReads.every((search) => new URLSearchParams(search).get('package') === PKG)).toBe(true); + expect(server.draftReads.length).toBeGreaterThan(0); + }); + + it('a pending draft in the package: the first save sends If-Match with the version the read served', async () => { + server.drafts.set(`view/${VIEW}@${PKG}`, { body: viewItem(['title']), version: 'hmac-sha256:pending' }); + await openEditAndSave(['title', 'status']); + expect(server.puts).toEqual([{ ifMatch: 'hmac-sha256:pending', ifNoneMatch: null, status: 200 }]); + }); + + it('a draft saved in the package while the panel was open: the first save is refused, not written over', async () => { + server.active.set(`view/${VIEW}`, viewItem(['title'])); + render( + + + , + ); + await flush(); + // Another editor's first save lands after this panel's load. + server.drafts.set(`view/${VIEW}@${PKG}`, { body: viewItem(['status']), version: 'hmac-sha256:theirs' }); + await act(async () => hook.current!.edit(viewItem(['title', 'status']))); + let saved!: Promise; + act(() => { + saved = hook.current!.save({ unmoved: () => true }); + }); + // The conflict dialog asks; the author keeps editing, and nothing is sent. + fireEvent.click(await screen.findByTestId('draft-conflict-cancel')); + await act(async () => saved); + expect(server.puts).toEqual([{ ifMatch: null, ifNoneMatch: '*', status: 409 }]); + expect(server.drafts.get(`view/${VIEW}@${PKG}`)?.body).toEqual(viewItem(['status'])); + expect(hook.current!.failure?.during).toBe('save'); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.createAppOpensInterfaces-11794.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.createAppOpensInterfaces-11794.test.tsx index 0c93d02e1b..fa09e973bc 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.createAppOpensInterfaces-11794.test.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.createAppOpensInterfaces-11794.test.tsx @@ -6,9 +6,10 @@ * button was pressed from. * * Navigation only. The create's one write — `save('app', NAME, skeleton, - * { mode: 'draft', packageId })` — is pinned byte for byte against - * `buildAppSkeleton`, so the card changes where the author lands and nothing - * the server receives. + * { mode: 'draft', packageId, ifNoneMatch: '*' })` — is pinned byte for byte + * against `buildAppSkeleton`, so the card changes where the author lands and + * nothing the server receives. (`ifNoneMatch` is objectui#11773's first-write + * pin, which every create sends.) * * The departure goes through the surface's own leave guard, the one a pillar * link uses (objectui#2600): an unsent edit on the pillar being left asks @@ -166,7 +167,7 @@ function expectTheCreateWriteUnchanged() { 'app', 'field_service', buildAppSkeleton('field_service', 'Field Service', [{ name: 'a_account' }]), - { mode: 'draft', packageId: 'app.a' }, + { mode: 'draft', packageId: 'app.a', ifNoneMatch: '*' }, ], ]); } @@ -185,7 +186,7 @@ describe('Create app opens the Interfaces pillar (objectui#11794)', () => { await waitFor(() => expect(pathnames[pathnames.length - 1]).toBe('/studio/app.a/interfaces')); expect(confirmSpy).not.toHaveBeenCalled(); // The Interfaces pillar resolved the app the create left as a draft. - await waitFor(() => expect(clientImpl.getDraft).toHaveBeenCalledWith('app', 'field_service')); + await waitFor(() => expect(clientImpl.getDraft).toHaveBeenCalledWith('app', 'field_service', { packageId: 'app.a' })); expectTheCreateWriteUnchanged(); }); @@ -218,7 +219,7 @@ describe('Create app opens the Interfaces pillar (objectui#11794)', () => { const toolbar = await renderOn('interfaces'); await createApp(toolbar); - await waitFor(() => expect(clientImpl.getDraft).toHaveBeenCalledWith('app', 'field_service')); + await waitFor(() => expect(clientImpl.getDraft).toHaveBeenCalledWith('app', 'field_service', { packageId: 'app.a' })); expect(pathnames).toEqual(['/studio/app.a/interfaces']); expect(confirmSpy).not.toHaveBeenCalled(); expectTheCreateWriteUnchanged(); diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesCreate-11823.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesCreate-11823.test.tsx index 37f788b65f..bba852128d 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesCreate-11823.test.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesCreate-11823.test.tsx @@ -186,7 +186,8 @@ describe('the Interfaces pillar creates a dashboard and a report (objectui#11823 // The item's draft: the spec's minimum, parsed by the spec. const [[, name, body, options]] = savesOf('dashboard'); expect(name).toBe('sales_overview'); - expect(options).toEqual({ mode: 'draft', packageId: 'com.acme.app' }); + // objectui#11773 — a create is the first-write pin. + expect(options).toEqual({ mode: 'draft', packageId: 'com.acme.app', ifNoneMatch: '*' }); expect(body).toEqual({ name: 'sales_overview', label: 'Sales overview', widgets: [] }); const parsedDash = DashboardSchema.safeParse(body); expect(parsedDash.success, JSON.stringify(parsedDash.error?.issues)).toBe(true); @@ -195,7 +196,9 @@ describe('the Interfaces pillar creates a dashboard and a report (objectui#11823 // no label, so it inherits the dashboard's. const [[, appName, appBody, appOptions]] = savesOf('app'); expect(appName).toBe('acme_app'); - expect(appOptions).toEqual({ mode: 'draft', packageId: 'com.acme.app' }); + // objectui#11773 — the app had no pending draft when the pillar loaded it, + // so the nav save that links the new item is the first-write pin too. + expect(appOptions).toEqual({ mode: 'draft', packageId: 'com.acme.app', ifNoneMatch: '*' }); expect(appBody.navigation).toEqual([ ...NAV, { id: 'nav_sales_overview', type: 'dashboard', dashboardName: 'sales_overview' }, @@ -237,7 +240,8 @@ describe('the Interfaces pillar creates a dashboard and a report (objectui#11823 const [[, name, body, options]] = savesOf('report'); expect(name).toBe('revenue_by_region'); - expect(options).toEqual({ mode: 'draft', packageId: 'com.acme.app' }); + // objectui#11773 — a create is the first-write pin. + expect(options).toEqual({ mode: 'draft', packageId: 'com.acme.app', ifNoneMatch: '*' }); expect(body).toEqual({ name: 'revenue_by_region', label: 'Revenue by region', diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesCreatePage-11823.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesCreatePage-11823.test.tsx index 7cbdfe3c03..94f04f045d 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesCreatePage-11823.test.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.interfacesCreatePage-11823.test.tsx @@ -182,7 +182,8 @@ describe('the Interfaces pillar creates a page and opens it on source plus live // The page's draft: an app page of the html kind, starting from the starter. const [[, name, body, options]] = savesOf('page'); expect(name).toBe('team_handbook'); - expect(options).toEqual({ mode: 'draft', packageId: 'com.acme.app' }); + // objectui#11773 — a create is the first-write pin. + expect(options).toEqual({ mode: 'draft', packageId: 'com.acme.app', ifNoneMatch: '*' }); expect(body).toEqual({ name: 'team_handbook', label: 'Team handbook', @@ -197,7 +198,9 @@ describe('the Interfaces pillar creates a page and opens it on source plus live // with its label: a page entry inherits none. const [[, appName, appBody, appOptions]] = savesOf('app'); expect(appName).toBe('acme_app'); - expect(appOptions).toEqual({ mode: 'draft', packageId: 'com.acme.app' }); + // objectui#11773 — the app had no pending draft when the pillar loaded it, + // so the nav save that links the new item is the first-write pin too. + expect(appOptions).toEqual({ mode: 'draft', packageId: 'com.acme.app', ifNoneMatch: '*' }); expect(appBody.navigation).toEqual([ ...NAV, { id: 'nav_team_handbook', type: 'page', pageName: 'team_handbook', label: 'Team handbook' }, diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.listViewInspector-11823.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.listViewInspector-11823.test.tsx index dfc84b172a..cff73d7aca 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.listViewInspector-11823.test.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.listViewInspector-11823.test.tsx @@ -189,9 +189,16 @@ describe('an object leaf\'s Properties panel edits its list view (objectui#11823 await addSortOnFirstField(panel); await waitFor(() => expect(viewSaves().length).toBeGreaterThan(0), { timeout: 8000 }); - const [, name, body, options] = viewSaves().at(-1)!; + const [, name, body] = viewSaves().at(-1)!; expect(name).toBe('showcase_task.default'); - expect(options).toEqual({ mode: 'draft', packageId: 'com.acme.app' }); + // objectui#11773 — the view had no pending draft when the panel loaded it, + // so its first save is the first-write pin; a later one is pinned to the + // receipt before it. + const sentOptions = viewSaves().map((call) => call[3]); + expect(sentOptions[0]).toEqual({ mode: 'draft', packageId: 'com.acme.app', ifNoneMatch: '*' }); + for (const later of sentOptions.slice(1)) { + expect(later).toEqual({ mode: 'draft', packageId: 'com.acme.app', ifMatch: 'v1' }); + } const parsed = ViewItemSchema.safeParse(body); expect(parsed.success, JSON.stringify(parsed.error?.issues)).toBe(true); expect(body).toMatchObject({ diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.sharedSelect-11865.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.sharedSelect-11865.test.tsx index c21232e61a..c1fe251ef6 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.sharedSelect-11865.test.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.sharedSelect-11865.test.tsx @@ -474,11 +474,14 @@ async function submitObject(): Promise { return JSON.stringify((mockClient.save as ReturnType).mock.calls[0]); } -/** [option label, the JSON text of the save the create made]. */ +/** + * [option label, the JSON text of the save the create made]. The create is the + * first-write pin, `ifNoneMatch: '*'` (objectui#11773). + */ const OWD_WRITES: ReadonlyArray = [ - [OWD_PRIVATE, '["object","visit",{"name":"visit","label":"Visit","sharingModel":"private","fields":{"name":{"type":"text","label":"Name"}}},{"mode":"draft","packageId":"com.test.crmext"}]'], - [OWD_PUBLIC_READ, '["object","visit",{"name":"visit","label":"Visit","sharingModel":"public_read","fields":{"name":{"type":"text","label":"Name"}}},{"mode":"draft","packageId":"com.test.crmext"}]'], - [OWD_PUBLIC_READ_WRITE, '["object","visit",{"name":"visit","label":"Visit","sharingModel":"public_read_write","fields":{"name":{"type":"text","label":"Name"}}},{"mode":"draft","packageId":"com.test.crmext"}]'], + [OWD_PRIVATE, '["object","visit",{"name":"visit","label":"Visit","sharingModel":"private","fields":{"name":{"type":"text","label":"Name"}}},{"mode":"draft","packageId":"com.test.crmext","ifNoneMatch":"*"}]'], + [OWD_PUBLIC_READ, '["object","visit",{"name":"visit","label":"Visit","sharingModel":"public_read","fields":{"name":{"type":"text","label":"Name"}}},{"mode":"draft","packageId":"com.test.crmext","ifNoneMatch":"*"}]'], + [OWD_PUBLIC_READ_WRITE, '["object","visit",{"name":"visit","label":"Visit","sharingModel":"public_read_write","fields":{"name":{"type":"text","label":"Name"}}},{"mode":"draft","packageId":"com.test.crmext","ifNoneMatch":"*"}]'], ]; describe('New object picks its record sharing with the shared Select (objectui#11865)', () => { From be52812b650c34cbf90fd2843d04b0822ccb5098 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:38:09 +0000 Subject: [PATCH 3/5] docs(data-objectstack): the save options' two preconditions, ifMatch and ifNoneMatch (objectui#11773) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- ...ewInspector.firstSavePinned-11773.test.tsx | 16 ++++--- packages/data-objectstack/README.md | 46 +++++++++++++++++++ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx b/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx index 377aed3730..4e8a770000 100644 --- a/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx @@ -117,11 +117,15 @@ const METADATA: MetadataContextValue = { const LEAF: Surface = { type: 'object', name: 'showcase_task', label: 'Tasks' }; -const hook = { current: null as ObjectListViewDraft | null }; +/** The hook's latest answer, handed out by the probe after each commit. */ +const hook = React.createRef() as React.MutableRefObject; -function Probe() { - hook.current = useObjectListViewDraft({ client, packageId: PKG, leaf: LEAF, publishNonce: 0 }); - return hook.current.conflictDialog; +function Probe({ hookRef }: { hookRef: React.MutableRefObject }) { + const draft = useObjectListViewDraft({ client, packageId: PKG, leaf: LEAF, publishNonce: 0 }); + React.useLayoutEffect(() => { + hookRef.current = draft; + }); + return draft.conflictDialog; } async function flush() { @@ -134,7 +138,7 @@ async function flush() { async function openEditAndSave(columns: string[]) { render( - + , ); await flush(); @@ -171,7 +175,7 @@ describe('useObjectListViewDraft — the first save after a load is pinned (obje server.active.set(`view/${VIEW}`, viewItem(['title'])); render( - + , ); await flush(); diff --git a/packages/data-objectstack/README.md b/packages/data-objectstack/README.md index a9ff75140c..4d3e5d5ee2 100644 --- a/packages/data-objectstack/README.md +++ b/packages/data-objectstack/README.md @@ -944,6 +944,52 @@ not apply it for you: the client cannot tell a served copy from an author who wrote `picklist` and `options` together, and that second case should stay the server's refusal, which says which key to delete. +## Saving Only Over the Version You Read + +Two editors of one metadata item overwrite each other unless each save says +which stored row it was built on. `MetadataClient.save` takes that precondition +as one of two options, never both: + +- `ifMatch: version`, sent as `If-Match`: save only over that row. The token is + the `version` the `/meta` door served for the same address (type, name, + `packageId`, and the draft or active lifecycle): from the item read + (`getDraft` hands back the envelope, `version` included), from a save + receipt, or as `currentVersion` on a `409 METADATA_CONFLICT` refusal. +- `ifNoneMatch: '*'`, sent as `If-None-Match: *`: save only where no row of that + lifecycle exists yet. A create sends it, and so does the first draft save of + an item with no pending draft, or one whose read served `version: null`. + +```ts +import { MetadataClient } from '@object-ui/data-objectstack'; + +const client = new MetadataClient({ baseUrl: '/api/v1' }); + +const read = await client.getDraft<{ item: Record; version?: string | null }>( + 'object', + 'account', + { packageId: 'com.acme.crm' }, +); +// No pending draft, or none at this address: the save creates the draft row. +// A server that serves no `version` at all leaves the save unpinned. +const pin = + read === null || read.version === null + ? { ifNoneMatch: '*' as const } + : read.version + ? { ifMatch: read.version } + : {}; +await client.save('object', 'account', { name: 'account', label: 'Customer' }, { + mode: 'draft', + packageId: 'com.acme.crm', + ...pin, +}); +``` + +Read at the package the save writes: a read that names no package serves a +package-bound draft with `version: null`, which says nothing about a save that +names the package. A save that sets both options throws before any request; +the door refuses the pair with a `400`. A save whose pin no longer holds is +refused `409 METADATA_CONFLICT`, and nothing is written. + ## User-Scoped State Adapter In addition to the main `DataSource` adapter, this package ships From cff84f170b111a99ebd29ed53db5fcc8215d1a0b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:39:03 +0000 Subject: [PATCH 4/5] fix(app-shell): a hook list load a package publish overtook records no pins (objectui#11773) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- .../src/views/studio-design/ObjectHooksPanel.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx index 2d4e627160..22ac5b8bf5 100644 --- a/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectHooksPanel.tsx @@ -145,6 +145,12 @@ export function ObjectHooksPanel({ React.useEffect(() => { forgetHookVersion(); }, [publishNonce, forgetHookVersion]); + // The publish a list load started under: a load still in flight when a + // publish lands read the drafts that publish drained, so it records no pins. + const publishNonceRef = React.useRef(publishNonce); + React.useLayoutEffect(() => { + publishNonceRef.current = publishNonce; + }); /* ─── objectui#11820 — the objects of this package, for the target picker ── * @@ -212,6 +218,7 @@ export function ObjectHooksPanel({ React.useEffect(() => { let cancelled = false; + const publishAtStart = publishNonceRef.current; setLoading(true); setError(null); (async () => { @@ -251,6 +258,7 @@ export function ObjectHooksPanel({ // built on: the install below does not replace that buffer either. const open = bufferRef.current; for (const [n, pin] of pins) { + if (publishNonceRef.current !== publishAtStart) break; if (!(open.dirty && open.hook === n)) recordHookRead('hook', n, packageId, pin); } } catch (e) { From 47e75737ec0b6bbc6a04074b73d2b0340dbcfb92 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:48:24 +0000 Subject: [PATCH 5/5] test(app-shell): the list-view pin reads the package address where a draft read is sent (objectui#11773) Claude-Session: https://claude.ai/code/session_01CsyMRsFJBKNEMa4oPY1b5z Co-authored-by: Claude --- ...ectListViewInspector.firstSavePinned-11773.test.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx b/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx index 4e8a770000..5eaebadc1b 100644 --- a/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectListViewInspector.firstSavePinned-11773.test.tsx @@ -157,18 +157,20 @@ beforeEach(() => { afterEach(cleanup); describe('useObjectListViewDraft — the first save after a load is pinned (objectui#11773)', () => { - it('a published view with no pending draft: the first save sends If-None-Match: *, the draft read named the package', async () => { + it('a published view with no pending draft: the first save sends If-None-Match: *', async () => { server.active.set(`view/${VIEW}`, viewItem(['title'])); await openEditAndSave(['title', 'status']); expect(server.puts).toEqual([{ ifMatch: null, ifNoneMatch: '*', status: 200 }]); - expect(server.draftReads.every((search) => new URLSearchParams(search).get('package') === PKG)).toBe(true); - expect(server.draftReads.length).toBeGreaterThan(0); + // The drafts ledger answered "none", so no `?state=draft` read was sent. + expect(server.draftReads).toEqual([]); }); - it('a pending draft in the package: the first save sends If-Match with the version the read served', async () => { + it('a pending draft in the package: the first save sends If-Match with the version the read, at the package, served', async () => { server.drafts.set(`view/${VIEW}@${PKG}`, { body: viewItem(['title']), version: 'hmac-sha256:pending' }); await openEditAndSave(['title', 'status']); expect(server.puts).toEqual([{ ifMatch: 'hmac-sha256:pending', ifNoneMatch: null, status: 200 }]); + expect(server.draftReads.length).toBeGreaterThan(0); + expect(server.draftReads.every((search) => new URLSearchParams(search).get('package') === PKG)).toBe(true); }); it('a draft saved in the package while the panel was open: the first save is refused, not written over', async () => {