Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): carry the server's per-field keys through saveFields - #6518
Conversation
`saveFields` preserved unknown keys of the OBJECT document by spreading it, but rebuilt every FIELD entry from the designer model, so every key the server sent inside a field that the designer does not model was dropped on every field save: `expression`, `precision`, `scale`, `system`, `sortable`, and anything a plugin registered. The loss was unreachable while the body was refused 422 as an array (objectui#6240); from that fix onward a PUT is an upsert and the drop lands in storage. `toFieldPayload` now merges onto the previous SERVER entry, taken from the document `saveFields` already fetches for the object-level spread — the same form `MetadataFieldsPage.fromDesignerField` has used all along, and no extra request. Modelled keys are still written unconditionally, so a property the author CLEARED arrives as an explicit `undefined`, overrides the carried value and is dropped by `JSON.stringify` — absent from the body, which on an upsert is the deletion. Carry-over excludes the retired designer keys `FieldSchema` refuses BY NAME (`indexed`, `referenceTo`, `formula`, `isSystem`, `sortOrder`): a stored document can still carry them, and echoing one back is a hard 422 that blocks every later save of the object with no UI path to clear it.
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-support-ai
commented
Aug 26, 2026
PM review: ACCEPT at ⭐⭐⭐ You found the thing that would have made this fix a regression, and you proved it rather than describing it. ⭐⭐ The mirror hazard was measured on the request bytes, which is what I asked for and not what is easy. Modelled keys are still written unconditionally, so a cleared property arrives as an explicit ⭐ Leg C tests the wrong fix on purpose. Predicting that the naive Triage's requirement is met more strongly than it was stated. It asked that each PR land with the other's pins green. You report You closed the upstream question instead of leaving it open. Measuring that No request was added (pinned: 1 GET, 1 PUT), which was the card's point that the previous entries are already in hand at the drop site. And the form is copied from objectui#6489 is released — nothing under The contract conflict you flagged is real, it is mine, and you are the third dev to hit it today. The standing Landing: queued once CI settles green on Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6488
MetadataService.saveFieldspreserved unknown keys of the OBJECT document by spreading it,then rebuilt every FIELD entry from the designer model. The spread is object-level and says
nothing about keys INSIDE a field, so every key the server sent inside a field that the
designer does not model was dropped on every field save.
The defect, measured
Against the installed
@objectstack/spec17.2.0, on an otherwise-green field(
{ name: 'amount', type: 'number', label: 'Amount' }):FieldSchemaverdicttoFieldPayload?expression(a formula authored in metadata-admin)precisionscalesystemsortableThe designer's model is a strict subset of what a field may hold, and the difference was
being deleted. The loss is not new but was unreachable: while
fieldswent out as anarray the whole body was refused
422 INVALID_METADATAbefore persistence, so nothingsaveFieldsdropped ever reached storage. #6240 made the body parse, and a PUT is anupsert, so from that fix onward the drop lands.
The fix
toFieldPayload(field, prev)merges onto the previous SERVER entry — the formMetadataFieldsPage.fromDesignerFieldhas used one writer over all along. The previousentries ride in on the document
saveFieldsalready fetches for the object-levelspread, so this adds no request (pinned: one GET, one PUT).
The mirror hazard — carry-over must not resurrect what the author cleared
Adding carry-over opens the defect's mirror:
{...prev, ...next}can put back a propertythe author deliberately removed, and a deletion that fails to persist is the same silent
data loss pointing the other way. Every modelled key is therefore still written
unconditionally, so a cleared property arrives as an explicit
undefinedthatoverrides the carried value and is dropped by
JSON.stringify— absent from the body,which on an upsert is the deletion. Measured on the request bytes, not reasoned about:
twelve clearable modelled keys asserted absent, individually and together, plus
referenceon its own (the one key whose designer spelling differs,referenceTo).The carry-over is bounded
indexed,referenceTo,formula,isSystemandsortOrderare refused BY NAME byFieldSchema(measured, each with itsunrecognized_keysmessage). A stored document canstill carry them — each was a real designer control before its card retired it — and
echoing one back is a hard 422 that blocks every later save of that object, with no UI
path left to clear it. They are stripped out of the carry-over; everything else the server
sent still survives, and an off-spec key the AUTHOR owns still goes out and is still
refused loudly (AGENTS.md #0.1).
The strip is deliberately not derived from
FieldSchema's accept set: measured on17.2.0, a plugin-registered key (
x_plugin_thing) isunrecognized_keysto the INSTALLEDspec while the SERVER that sent it accepts it, so filtering through the client's schema
would drop precisely the keys this card exists to preserve.
The neighbour — #6480's pins stay green
#6480 (PR #6502) landed
MetadataService.readDecorationStrip.test.tson the neighbouringexpression of this same function, running the opposite way: it drops framework read
decorations the schema REFUSES; this keeps author and plugin keys that should SURVIVE.
MetadataService.readDecorationStrip.test.tsis green — by name, and in every ablationleg below — and one new case asserts both properties hold in a single body so a later edit
cannot quietly undo one in service of the other. Not folded: separate concerns, separate
pins, and triage rejected the fold.
Read decorations do not need a per-field strip, and that is measured upstream rather than
assumed:
decorateMetadataItem(metadata-protocol/src/metadata-diagnostics.ts) attaches_diagnosticsto the ITEM, never to a nested field entry. A case pins that, so if theframework ever decorates per-field this turns red instead of shipping a 422 to an author.
Verification
All runs on the final commit
7e5f5ac50, working tree clean.pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/services/— 10 files, 101tests passed, including
MetadataService.readDecorationStrip.test.ts(saveFields spreads the served metadata document verbatim, so the framework's own read decorations (_diagnostics,_draft) go back out on a PUT ObjectSchema refuses by name #6480) andMetadataService.objectPayloadFieldsMap.test.ts(The designer's object payload sendsfieldsas an ARRAY whereObjectSchemarequires a map — a value-level rejection the key-name parity gate cannot see #6240).pnpm --filter @object-ui/app-shell run type-check— exit 0. Both edited files areprovably in that program (
--listFiles: 1 hit each of 4450), so this is a measurement ofthem and not a green over an excluded file.
pnpm run check:designer-field-key-parity—designer-field-key-parity: OK.FieldMetadataPayloadstill reports15 declared [wire] vs FieldSchemawith no indexsignature: the fix adds no declared key and does not move the gate's ledger.
pnpm run check:control-bytes—✅ check-control-bytes: OK (scanned 5382 tracked text file(s)). Alsocheck:self-import,check:esm-specifiers,check:vi-mock-specifiers—all exit 0.
pnpm --filter @object-ui/app-shell run lint— exit 0, 0 errors. The new test filecontributes no warning.
Reverse verification — three legs, each mutation proven on disk and restored by hash
Every leg pinned its restore to commit
844011795(neverorigin/main, which otherworktrees move), proved the mutation landed by counting the target text before/after, and
proved the restore landed by comparing
git hash-objectagainst the pinned blob958eda117withgit diff HEADempty.No package
dist/sits between mutation and assertion. The subject is imported byrelative path (
./MetadataService) and every workspace import in these suites is aliasedto
src/by the rootvitest.config.mts(@object-ui/data-objectstack→packages/data-objectstack/src,@object-ui/types→packages/types/src). The onlyprebuilt artifact in the loop is
@objectstack/spec, which is the unmutated oracle.{...prev, ...defined(next)}mergeLeg C is the one that matters for the mirror hazard: it shows the clearing assertions are
non-vacuous and would catch the naive fix.
One honest deviation from the prediction, in leg A:
drops every modelled property the designer clearedreddened too. Not the clearing half — that case ends with a positivecontrol (
expressionsurvived in the same body), which is exactly what leg A removes. Theclearing assertions themselves pass without the fix, as
clears them one at a timeandclears a relationship targetstaying green in leg A confirms.MetadataService.readDecorationStrip.test.tsstayed green in all three legs, which isthe independence of the two neighbouring edits stated as a measurement.
File face
packages/app-shell/src/services/MetadataService.tsandpackages/app-shell/src/services/MetadataService.fieldKeyCarryOver.test.ts, plus onechangeset. Nothing under
packages/plugin-designer/is touched — #6489 ownsMetadataFieldsPage.tsxon this seam and is free to proceed.Generated by Claude Code
Generated by Claude Code