Repository navigation
Commit 2dec305
fix(app-shell): Studio draft saves send the version they were built on, and a stale save opens a reload / overwrite dialog (objectui#11773) (#11826)
Part of #11773 — the client half. The card stays open for the server
half, named in the section on it below.
Clause-②: no
## What changes
Every Studio draft save of an existing metadata item now sends
`If-Match` with the `version` its editor's previous save received, and
holds the new receipt's `version`. When the `/meta` draft door refuses a
stale version with `409 METADATA_CONFLICT`, the editor opens a conflict
dialog with three choices:
- **Reload saved version.** The editor re-runs its own load. Its unsaved
edits on screen are dropped.
- **Overwrite…** A second, explicit confirmation follows. The same body
is then sent again without `If-Match`, and it wins.
- **Keep editing.** Nothing is saved, and the refusal is shown in the
editor's error strip. The stale version is kept, so the next save is
refused again and cannot slip through.
The door's other 409, `DESTRUCTIVE_CHANGE`, is judged before the version
and passes through the guard untouched to each editor's existing
confirmation flow.
One guard per editing buffer, `useDraftSaveGuard` in the new
`views/metadata-admin/DraftConflictDialog.tsx`, with the dialog beside
it. A guard belongs to one buffer, not to an item: two surfaces in one
tab that each hold their own copy of an item are two editors, and
sharing one version between them would let the second one's stale copy
through. The rules, in that file's header:
1. A draft save sends the version this guard holds for the same item
(type, name, package), and nothing otherwise.
2. A save that lands holds the receipt's `version`.
3. A buffer installed from a server read (a load, an item switch, a
reload after a publish or a discard) holds no version, because the read
serves none (measured below). The caller says so with `forget()`. The
read-back of the guard's OWN save does not forget.
4. A `METADATA_CONFLICT` opens the dialog (reload / overwrite / keep
editing, as above).
5. Saves through one guard run one at a time, so each one sends the
version the previous one received. An autosave never conflicts with the
explicit save (column reorder, enable switch) the same buffer sent a
moment earlier. This is the self-conflict risk named in Zone 2 item 2.
Creates send no `If-Match`. The door cannot express "expect no row" over
HTTP (below), so a create keeps calling the client directly.
## Measured first (Zone 2 item 1)
Published `@objectstack/cli` 17.7.0 was installed into a scratch
directory. `rest`, `metadata-protocol`, `runtime` and `spec` all read
17.7.0, the release this repo's lockfile resolves. It booted a
one-object probe app with `objectstack dev --seed-admin --fresh
--no-watch -p 4773`, signed in as the seeded admin, and created a
writable authoring package `com.probe.studio`. Then it drove the
`/api/v1/meta` door with curl. Readings, 2026-10-07T16:38Z to 16:41Z:
| Step | Request | Answer |
|---|---|---|
| create | `PUT
/meta/object/pst_ticket?mode=draft&package=com.probe.studio`, no
`If-Match` | 200
`{"success":true,"version":"hmac-sha256:7102ff…f950","seq":1,"state":"draft","message":"Saved
object 'pst_ticket' (env-wide, state=draft) [seq=1]"}`. No `ETag`
header. |
| draft read | `GET /meta/object/pst_ticket?state=draft&package=…` | 200
`{type, name, sortability, item}`. No version key in the envelope or the
item, and no `ETag` header. |
| active read (cached path) | `GET /meta/object/occprobe_ticket` |
`ETag: "2d68dba9"`. That is the cache validator, not a version token (8
hex digits). |
| editor A, fresh token | PUT draft, `If-Match:` the create's version |
200, new version (`seq` 2) |
| editor B, stale token | PUT draft, `If-Match:` the create's version
again | **409** `{"error":"object/pst_ticket has been modified since you
loaded it. The version token sent is not the current version (current is
hmac-sha256:b375…2f14).","code":"METADATA_CONFLICT"}`. The draft still
held A's `pluralLabel`. |
| quoted token | `If-Match: "TOKEN"` | 200. The quotes are stripped. |
| garbage or empty token | `If-Match: nope`, or an empty `If-Match` |
409 `METADATA_CONFLICT` |
| after a publish | `POST …/publish`, then a PUT draft with the last
draft version or with the publish receipt's version | 409
`METADATA_CONFLICT`, "current is null". The publish dropped the draft
row, so any token is refused. |
| first draft after a publish | PUT draft, no `If-Match` | 200 |
| destructive, with any token | PUT draft dropping a field that holds
data | 409 `{"error":"… would drop or transform existing data
…","code":"DESTRUCTIVE_CHANGE","issues":[…]}`, both with a stale token
and with a fresh one |
| destructive plus force, stale token | `?force=true`, stale `If-Match`
| 409 `METADATA_CONFLICT`. Destructive is judged first and the version
second. |
| data door | `GET /api/v1/data/sys_metadata` | The draft row's
`checksum` column is served keyed, and equals the last receipt's
`version`. Not used: it would mean re-deriving the door's served-row
resolution in the client. |
So:
- **The server does enforce `If-Match` on `mode=draft` writes.** Zone
2's stop condition did not fire.
- **The token is the save receipt's `version`, a keyed digest, and only
the receipt serves it.** The draft read serves none. The client docblock
on `MetadataClientSaveOptions.ifMatch` ("the `checksum` returned by the
last read") names a token no `/meta` read serves. That is reported as a
finding, and `packages/data-objectstack` is untouched here.
- **The two 409s are told apart by `code`.** `isDraftVersionConflict`
reads `status` plus `code` off the client's parsed error and never reads
the prose.
- **Overwrite does not re-send "the server's current token".** The 409
body carries it only inside the error sentence, and the door serializes
no structured field for it. The guard does not parse prose, so overwrite
re-sends without `If-Match` after the confirmation. That is still a
last-writer-wins write: a third writer landing between the refusal and
the confirmed overwrite is overwritten. The author chose that write
knowing the draft had moved. A structured current version on the 409 is
part of the server finding below.
## Every save call site (Zone 2 item 6)
Read on `9990f9e` by `git grep "\.save("` over `packages/app-shell/src`,
tests excluded. Sites are named by function, not by line.
| File and function | Decision |
|---|---|
| `StudioDesignSurface` Data pillar `doSave` (object autosave) |
**OCC-guarded.** Its load forgets. Reload re-runs the load for the open
object. |
| `StudioDesignSurface` Data pillar `doReorderFields` (grid column drag)
| **OCC-guarded**, on the same guard as the autosave, so the two are
serialized |
| `StudioDesignSurface` Data pillar `doCreateObject` | **Create.** No
`If-Match`. |
| `StudioDesignSurface` Automations pillar `doSave` (flow autosave) |
**OCC-guarded.** Its load forgets. |
| `StudioDesignSurface` Automations pillar `toggleEnabled` |
**OCC-guarded**, same guard as the flow autosave. On a refusal the
existing rollback of the optimistic flip still runs. |
| `StudioDesignSurface` Automations pillar `doCreateFlow` | **Create.**
|
| `StudioDesignSurface` Interfaces pillar `doSave` (page, dashboard and
other leaves) | **OCC-guarded.** The leaf load forgets, including the
empty-buffer branch. |
| `StudioDesignSurface` Interfaces pillar `doNavSave` (app navigation) |
**OCC-guarded.** The app load re-reads after every draft save in the
package. The re-read that follows this pillar's own nav save keeps the
version. Any other install forgets it, and so does an install after a
publish. |
| `StudioDesignSurface` shell `doCreateApp` | **Create.** |
| `StudioDesignSurface` Access pillar permission create
(`buildPermissionSkeleton`) | **Create.** |
| `ResourceEditPage` `doSave` | **OCC-guarded** in edit mode, and a
**create** in create mode. Its load effect forgets, and so do
`doPublish`, `doDiscardDraft` and `doReset`. Its post-save read-back is
the echo of its own save and keeps the version. The destructive-change
dialog is unchanged and is a separate dialog. |
| `PermissionMatrixEditor` `doSave` | **OCC-guarded** at the package
door (`mode: 'draft'`). The environment door's live write passes through
unpinned, as before. That is a non-draft write, outside this card. |
| `ObjectHooksPanel` `save` | **OCC-guarded.** A package publish forgets
(the panel now receives `publishNonce`). Its list re-read after its own
save keeps the version. |
| `ObjectHooksPanel` `addHook` | **Create.** |
| `PackageOwdOverviewPanel` `doSave` | **Not guarded.** It reads each
object fresh, inside the same click, immediately before patching only
the two OWD keys. It holds no long-lived buffer of the document, and the
read serves no version to pin. The lost-update window is that one
read-to-write round trip. A long-lived editor of the same object (the
Data pillar) is protected by its own version: its next save is refused
after this panel moved the draft. |
| `EmbeddedItemEditor` `doSave` | **Not a draft save.** It passes no
`mode`, so it is a live write of the parent after a fresh `layered` read
in the same click. |
| `DatasourceResourcePage` (external object import) | **Not a draft
save.** It passes no `mode`: a live create of the imported object. |
| `runtime-metadata-persistence` `createRuntimeMetadata` | **Create.** |
| `runtime-metadata-persistence` `persistRuntimeMetadata` | **Not
guarded here.** Its callers hold the buffer: the console's runtime view
editor (`ObjectView`'s view-config Save) and `ReportView`'s Save. Both
are explicit-Save editors outside Studio and outside this card's file
surface. Pinning them means giving those callers a guard. That is named
as the remaining client follow-up on this card, not done here. |
Outside the claimed surface and not draft saves, recorded for
completeness: `preview/UnpublishedAppBar` (a live PUT of the app, the
ADR-0045 visibility flip) and `metadata-admin/external/api`
`importObjectDraft` (a live PUT create).
## What this does not fix: the server half
The card's own reproduction is not fixed by this PR. Tabs A and B both
open the item, then each saves once. B's first save has no version to
send, because the draft read serves none on 17.7.0, so it is still
last-writer-wins. What changes is that the loss is no longer permanent
and silent. A's next save is refused with the dialog (A holds a version
B's write moved), so A sees it and chooses. After each editor's first
save, every later save is protected.
Closing the first-save window needs the server to:
- serve the version on the `/meta` item read (a body field declared in
`GetMetaItemResponseSchema`, or an `ETag` equal to the token) for
`state=draft` and stored-row reads;
- let a client pin "no draft yet" over HTTP (for example `If-None-Match:
*`), for the first draft after a publish and for creates;
- name the current version structurally on the `METADATA_CONFLICT` body,
not only in the sentence.
That is reported to the seat as a cross-repo finding. With the server
half, the guard also records the version from each read. That is a
one-line change at each load that today calls `forget()`.
## Tests
Server double modelled on the measured door. The unit suite runs the
**real** `MetadataClient` over a fetch double. The Studio and designer
suites throw refusals parsed by the real client's error parser.
- `views/metadata-admin/DraftConflictDialog.test.tsx` (12 tests). Token
advance; serialized back-to-back saves; `forget()`; one version per item
and package; non-draft passthrough; reload (plus a queued save dropped
on reload); overwrite; keep editing (refused again); after-publish
control; `DESTRUCTIVE_CHANGE` passthrough with the forced retry keeping
the version; the code-not-prose predicate.
-
`views/studio-design/StudioDesignSurface.draftVersionConflict-11773.test.tsx`
(5 tests, two mounted Data pillars over one server). One editor's
consecutive autosaves all succeed. Two editors: the stale save gets the
dialog and the other editor's change survives, then reload, then
overwrite. A create sends no `If-Match`.
-
`views/metadata-admin/ResourceEditPage.draftVersionConflict-11773.test.tsx`
(3 tests). Token advance. The conflict dialog, not the destructive one.
Control: a destructive change still opens its own confirmation, and its
forced retry keeps the version.
Runs (all through the shared verify lock; seconds are shared-box
readings):
- `pnpm --filter @object-ui/app-shell type-check` at `16c92cf`: echoed
`tsc --noEmit && tsc -p tsconfig.test.json`, `TYPECHECK_EXIT=0`.
- `pnpm exec vitest run packages/app-shell/ --maxWorkers=3` at
`16c92cf`: `Test Files 1061 passed | 1 skipped (1062)`, `Tests 10383
passed | 9 skipped (10392)`, `VITEST_EXIT=0`.
- The three new files: `Test Files 3 passed (3)`, `Tests 20 passed
(20)`.
- `pnpm exec eslint` over the 9 touched `.ts`/`.tsx` files: 0 errors.
Per-file warnings equal the base or lower: `StudioDesignSurface.tsx`
drops from 17 to 14, because three `useCallback`s gained their missing
`packageId`. The new `DraftConflictDialog.tsx` carries 3
`react-refresh/only-export-components` warnings, from exporting the
guard and its hook beside the component (the provider-plus-hook shape
several app-shell files already use). This narrowing is a measurement,
not a skipped run. The population is the 9 files, read from `--format
json`. The config is not type-aware (no `parserOptions.project`) and no
repo rule reads other files, so this diff cannot move a verdict on an
untouched file. The repo-wide lint is CI's.
- Gates, each run on the tree at `16c92cf`, each exit 0, with the gate's
own line quoted: `check:control-bytes` "OK (scanned 7783 tracked text
file(s)…)". `check:test-path-roots` "OK". `check:changeset-claims` "No
pending changeset names a file this change touches."
`check:pending-changeset-literals` "No test source names a pending
changeset." `check:i18n-keys` "Every in-scope call-site key resolves…".
`check:i18n-drift` "No designer-table en value changed in this range."
(10 keys added). `check:i18n-designer-parity` "Every en row has a zh
row, and every shared row carries the same placeholders."
`check:new-line-citations` "VERDICT new-cross-file-line-citations: 0 new
citation(s)". `check:vi-mock-specifiers`, `check:vi-mock-inherit` and
`check:vi-mock-override-shape` "OK". `check:metadata-write-doors` "OK 17
metadata write door(s) derived…". `check:unreferenced-sources` "OK Every
shipped source file in every covered package is reachable."
`check-changeset-presence.mjs` "9 source file(s) of 1 released
package(s) changed, and this change declares 1 changeset(s)".
`check:i18n-dead-keys` (a report) lists none of the new keys.
### Ablation: stop sending `If-Match`
The fix was committed first (`16c92cf`). Then `node
../objectstack/scripts/ablation-replace.mjs` replaced `pinned ? {
...options, ifMatch: pinned } : options` in `DraftVersionGuard.run` with
`{ ...options } /* ABLATED-11773: ifMatch never sent */`. The tool's own
evidence: anchor `x1 -> x0`, replacement `x0 -> x1`, blob `866eec3fa710
-> 5e0c9f9dfa9e`. Inside the locked run, `MARKER_COUNT=1
ANCHOR_COUNT=0`. Result: **`Tests 16 failed | 4 passed (20)`**. The
two-editor pin times out waiting for the dialog. The 4 that stayed green
never depend on a pin: one version per item, `forget()`, non-draft
passthrough, and the code-not-prose predicate. The restore was proven by
the tool: blob after restore == blob at `HEAD` (`866eec3fa710`), and
`git diff HEAD` empty. The direction was red, as expected. A first
attempt was refused by the tool before it ran anything, because its
replacement (`options`) was a substring of the anchor and the count
could not move. That attempt was a no-op, restored and proven, and is
not a reading.
## Acceptance notes
- The dialog offers reload and overwrite, which is the triage direction.
The card's "review" (a diff of theirs against mine) is not built.
- A dev server restart re-keys versions when no crypto provider is
registered (the server's ephemeral key). The first save after a restart
is then refused once with the dialog, by the server's design.
- A draft dropped by a path that does not reload this editor (a discard
from the Packages page, a publish from another tab) leaves the editor
holding a version of a row that no longer exists. Its next save is
refused with the dialog, and reload resolves it. The refusal was
measured after a publish; after a discard it follows from the same rule
and was not measured separately.
- The pillars read the draft with `getDraft(type, name)` and no
`packageId`, while they save with the package. That is unchanged here
and is not measured.
Implemented by the dispatched os-dev subagent of the `domain:ui#3` seat,
session `https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 2b30d39 commit 2dec305
10 files changed
Lines changed: 1430 additions & 25 deletions
File tree
- .changeset
- packages/app-shell/src/views
- metadata-admin
- studio-design
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
Lines changed: 302 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
0 commit comments