Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-designer): build the Field Designer's fields map as own properties and refuse what it cannot carry - #6520
Conversation
…properties and refuse what it cannot carry `MetadataFieldsPage.handleFieldsChange` keyed its `fields` map by blind assignment inside a bare `for` loop, which failed silently three ways — all measured on the installed `@objectstack/spec` 17.2.0: - a field named `__proto__` invoked the prototype setter instead of creating a key, so it vanished from the serialised PUT body while the spec (key rule `/^[a-z_][a-z0-9_]*$/`) stood ready to accept it; - a nameless field keyed as the literal string `"undefined"`, which `ObjectSchema.safeParse` accepts — parsed, stored, and read by nothing; - two fields sharing a name collapsed into one entry, the later silently replacing the earlier. The map is now built through `Object.fromEntries`, and the nameless and duplicate lists are refused before `client.save` runs, so a refused list issues no request. The refusal lands in the page's existing error surface rather than being thrown past a fire-and-forget caller. Ported from the sibling object writer app-shell `MetadataService.toFieldsMap` (objectui#6240), down to the refusal wording, so the two writers of the objectui#5761 parity family cannot drift. `fromDesignerField`'s carry-over semantics are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
✅ 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 ⭐⭐⭐ objectui#6524 corrects my dispatch order, and it is the sharpest finding of the round. I told you to "mirror the pin pattern" in You mirrored the pin's intent — a computed The fence held, and the comments show you understood why it existed. ⭐⭐ You wrote the pin first and ran it against unmodified source. Seven red, five green, and the ⭐⭐ The throwaway wire probe is what makes this a measurement of the real harm. Capturing the actual PUT bytes on the unfixed page — The ablation predicted before it measured: ⭐ The divergence from the sibling writer is named rather than hidden, and it is forced. The refusal raises inside the page's existing save ⭐ And the bounded hardening you did NOT do is recorded properly. objectui#6522 is the same defect class one level up, with a worse consequence. Landing: Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6489
Port of the three refusals objectui#6240 landed in the sibling object writer (app-shell's
MetadataService.toFieldsMap) intoMetadataFieldsPage, the objectui#5761 parity family's other writer.fromDesignerField's carry-over semantics are untouched — this card is the map construction around it, and objectui#6488 (PR #6518) copied that builder's exact form into app-shell so the two cannot drift.File face — exactly three files
packages/plugin-designer/src/MetadataFieldsPage.tsxtoFieldsMap;handleFieldsChangecalls it inside its existing savetrypackages/plugin-designer/src/MetadataFieldsPage.fieldsMapKeying.test.tsx.changeset/6489-designer-fields-map-keying.md@object-ui/plugin-designer: patchThe three hazards, measured on the installed
@objectstack/spec17.2.0The premise was re-verified on merged
mainbefore writing:MetadataFieldsPage.tsx:293still readnextFields[f.name] = fromDesignerField(f, prevFields[f.name]);in a bareforloop.1.
__proto__was eaten by the assignment itself.map['__proto__'] = definvokes the prototype setter instead of creating a key, and__proto__matchesObjectSchema.fields' key rule/^[a-z_][a-z0-9_]*$/— so the spec stood ready to accept the field the client threw away. Measured (computed key, see the fixture note below):ObjectSchema.safeParse({ …, fields: { ['__proto__']: { type: 'text', label: 'P' } } })returnssuccess = true. This is what makesObject.fromEntriesload-bearing rather than stylistic.2. A nameless field keyed as the literal string
"undefined".ObjectSchema.safeParse({ …, fields: { undefined: … } })returnssuccess = true, so it parsed, stored, and had no reader anywhere. Captured off the unfixed page, the actual request bytes:3. Duplicate names collapsed silently. Two designer fields called
amountproduced one entry — the later one, verbatim from the unfixed page:One deliberate difference from the sibling writer
MetadataServicethrows to its caller because it is a service. Here the caller isonFieldsChange={(next) => { void handleFieldsChange(next); }}— fire-and-forget — so a throw would become an unhandled rejection and show the author nothing: the same silent failure this card exists to end. The refusal therefore raises inside the page's existing savetry, landing in themetadata-fields-page-errorsurface with the offending index named. The property both writers share is the one that matters: it raises beforeclient.save, so a refused list issues no PUT at all (pinned:expect(puts).toHaveLength(0)).Reverse verification — one leg per refusal
The pin file was written first and run against the unmodified source, so each refusal is shown failing before and refused after. No
dist/sits between mutation and assertion: the test imports./MetadataFieldsPageby relative specifier, and the root vitest config aliases every@object-ui/*package to itssrc.Tests 7 failed | 5 passed (12)— the 4 instrument cases and the C0 control green, every hazard case redTests 12 passed (12)The
__proto__red was the disappearance itself:AssertionError: expected [ 'name', 'amount' ] to deeply equal [ 'name', '__proto__', 'amount' ].Ablation —
Object.fromEntries→ assignment, guards keptMirrors the sibling pin's ablation. Prediction stated before running: exactly the two
__proto__cases red, the other ten green.The mutation is proven on disk by grep counts of both the removed and the injected text plus a blob-hash change, and the restore by hash equality against the base blob — not by any exit code. The script carried a
trap … EXIT INT TERMwith absolute paths as a crash-path convenience; the hashes are the proof.Two fixture mechanics this depends on
{ __proto__: v }in an object literal SETS THE PROTOTYPE (Annex B.3.1) — it does not add a key. Every fixture here spells it['__proto__'], a computed key. A fixture written the plain way carries zero own keys and passes for the wrong reason. Filed as a finding against the sibling pin's instrument line, which is written the plain way (finding(app-shell): the__proto__instrument assertion inMetadataService.objectPayloadFieldsMap.test.tsmeasures{}— the object literal sets the prototype instead of adding a key #6524) — its claim is true, but that assertion does not measure it.JSON.parseDOES define an own__proto__property, which is why assertions on captured bytes read the key back honestly, and why the round-trip case (a stored__proto__field loads and re-saves with its unknown server keys intact) is meaningful.One bounded same-class hardening, named rather than slipped in
toFieldsMapreads the previous definition as an own property (Object.prototype.hasOwnProperty.call(prevFields, name)) instead ofprevFields[name]. Same defect class, in the same expression being rewritten: the bare read answers out ofObject.prototypefor the two spec-legal names that live there (__proto__,constructor). Measured, it is behaviour-identical today —carryOverspreads whatever it gets and both prototype values spread to{}— so no test can pin it, and none pretends to. It is here because that harmlessness iscarryOver's to lose.Verification run
All heavy runs went through the shared verify lock (
os-verify-lock.sh); every VERDICT line readcommand-exit N, none wasUNLOCKEDor99. Verdicts are quoted from each gate's own output, not from$?after a pipe.pnpm exec vitest run packages/plugin-designer/Test Files 14 passed (14)·Tests 101 passed (101)pnpm --filter @object-ui/plugin-designer run type-check(tsc --noEmit && tsc -p tsconfig.test.json)--listFilesconfirms both changed files are program inputs (1 hit each), so this is not the "typecheck excludes tests" no-oppnpm --filter @object-ui/plugin-designer run lint0 errors, 67 warnings— 50 files linted (--format json), all 67 warnings pre-existing in other files; both changed fileserr=0 warn=0check:designer-field-key-paritydesigner-field-key-parity: OKcheck:control-bytesOK (scanned 5386 tracked text file(s))— includes the new file (5385 before it was staged); plus a directgrep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over both files, no hitscheck:vi-mock-specifiersOKcheck:phantom-depsOK — every in-scope import is declared by the package that publishes itcheck-changeset-presence/check-changeset-no-major2 source file(s) of 1 released package(s) changed … declares 1 changeset(s)·No changeset declares a major bumpPrerequisite for the two
tscprograms:turbo run build --filter='@object-ui/plugin-designer^...' --concurrency=2(14 tasks, 2m32s) —tsconfig.test.jsondrops the source-treepaths, so without the built.d.tsclosure that program measures nothing.Declared narrowing: repo-wide
pnpm lintandpnpm testwere not run locally — CI runs both in full. The narrowing is at package granularity, and it is a measurement rather than a gap: (1) the universe comes from eslint's own config resolution (turbo run lintis per-packageeslint .), (2) the count is eslint's own — 50 files, (3) the rooteslint.config.jsdeclares noparserOptions.project/projectService, i.e. type-aware linting is off, so this diff cannot move the verdict on any file it does not touch.Union re-run after the final commit, on
git rev-parse --short HEAD=835396401: the package suite,type-check, package lint and the five repo gates above were all run on that tree.Out of scope, filed not folded
MetadataObjectsPagekeys its delete-detection lookup by blind assignment — an object namedconstructorcan never be deleted #6522 —MetadataObjectsPage.handleObjectsChangebuilds itsnextByNamelookup by the same blind assignment. Its deletion detection is therefore prototype-reachable: measured, an object namedconstructoror__proto__reads as "still present" soclient.resetnever fires, and both parse green as object names.__proto__instrument assertion inMetadataService.objectPayloadFieldsMap.test.tsmeasures{}— the object literal sets the prototype instead of adding a key #6524 — the sibling pin's instrument line writesfields: { __proto__: … }as a plain literal, so it measures{}with an odd prototype rather than the key. The claim it makes is true (verified here with a computed key); the assertion is not the one that shows it.Generated by Claude Code