Skip to content

fix(app-shell): strip the retired referenceTo / isSystem field keys on read (#6519) - #6529

Merged
os-support-ai merged 1 commit into
mainfrom
claude/issue-6519-retired-field-key-strip
Aug 26, 2026
Merged

fix(app-shell): strip the retired referenceTo / isSystem field keys on read (#6519)#6529
os-support-ai merged 1 commit into
mainfrom
claude/issue-6519-retired-field-key-strip

Conversation

@claude

@claudeclaudeBot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Fixes#6519

What changed

previews/object-fields-io.ts is the single read door for draft.fields across the whole object designer (inspector, form designer, design surface, settings / validations / API panels), and writeFields writes each def back verbatim. Its strip set named ONE key while FieldSchema refuses five by name, so a stored field carrying a sibling round-tripped straight back out to PUT /api/v1/meta/object/:name.

RETIRED_FIELD_KEYS now reads [indexed, referenceTo, isSystem]. Same refusal objectui#4644 applied to indexed in this same file, applied to the siblings that were left open.

File face — nothing under packages/plugin-designer/, so #6489 keeps sole ownership of MetadataFieldsPage.tsx:

  • packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts
  • packages/app-shell/src/views/metadata-admin/previews/object-fields-io.retiredKeys.test.ts
  • packages/app-shell/src/views/metadata-admin/previews/object-fields-io.spec-keys.test.ts
  • .changeset/6519-retired-field-key-strip.md

The premise was verified per key, and it answered differently for each

The card asked whether each surviving key can actually appear in a draft this door reads. It reads documents fetched through MetadataClient.get(object, name) — the same store the two sibling writers write to — so the question is whether a shipped build ever wrote the key.

keycan a draft carry it?evidencestripped
referenceToyesemitted by BOTH writers until objectui#6041 — MetadataService.toFieldPayload (referenceTo: field.referenceTo) and MetadataFieldsPage.fromDesignerField (referenceTo: designed.referenceTo)yes
isSystemyesdeclared on MetadataFieldsPage's ServerFieldSchema and read back (isSystem: raw.isSystem) until objectui#6044yes
formulayes, but the strip costs more than it fixessee belowno
sortOrdernoobjectui#6045's own commit: nothing on the tree ever populated a field-level one, so toFieldPayload emitted sortOrder: undefined and JSON.stringify dropped it — "the key never reached the wire"no

Nothing is lost on the way out: reference and system are real FieldSchema keys, are NOT stripped, and ride through untouched.

Why formula is not in the list, measured rather than reasoned

ObjectFieldInspector is the platform's sanctioned migration surface for that key — the legacy value seeds its linting CEL editor (readPredicate(def.expression ?? def.formula)) and the first edit commits expression and clears the alias. Stripping at the read door empties that editor. With formula added to the list:

FAIL packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.test.tsx
> commits edits to `expression` and migrates the legacy `formula` key
expect(element).toHaveValue(record.amount) Received: ""
Tests 1 failed | 42 passed (43)

objectui#6043 refused a blind rename in plugin-designerprecisely because that linting editor exists; a strip here would discard what the ruling preserved, on the next save, with no author-visible surface. That leaves the card's harm live for that one key, which is a maintainer trade rather than a dev's — raised with both options and a four-axis recommendation as #6526, not taken here.

Both absences are recorded in full on the tombstone, so the next reader does not "finish the set" defensively.

Reverse verification

Predicted RED, observed RED, on the emitted body rather than a helper's return value: the new cases in object-fields-io.spec-keys.test.ts build the object document PUT validates and parse it with the real ObjectSchema. For each stripped key, the un-round-tripped draft is refused at path fields.amount with unrecognized_keys (the 422's own shape), and the round-tripped body parses green. Controls: the document is otherwise accepted, and the existing negative controls (bogus key refused; the instrument is strict) still stand — if any control passes, the result is void.

The formula collision above is the second measured direction: predicted red, observed red, and it is what shrank the fix.

No package dist/ sits between mutation and assertion: the subject is imported relatively (./object-fields-io), and the only prebuilt artifact in the loop is @objectstack/spec, the unmutated oracle. (The dependency closure was built once — pnpm --filter @object-ui/app-shell^... build — because type-check needs it in a fresh worktree.)

Tests and gates — all on final commit 273a7d4d1, tree clean

  • pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/views/metadata-admin/previews/ packages/app-shell/src/views/metadata-admin/inspectors/ -> Test Files 101 passed (101), Tests 1144 passed | 1 skipped (1145)
  • pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/views/studio-design/ .../clientValidation .../ResourceEditPage (the read door's other consumers) -> Test Files 51 passed (51), Tests 363 passed (363)
  • pnpm --filter @object-ui/app-shell run type-check -> exit 0. Not the not-measured case: tsc -p tsconfig.test.json --listFiles shows 1 hit each for the three edited files out of 4458.
  • pnpm check:designer-field-key-parity -> designer-field-key-parity: OK (this seam's gate, objectui#5761 coverage note 3)
  • pnpm check:control-bytes -> OK (scanned 5395 tracked text file(s); skipped 85 binary); check:self-import, check:esm-specifiers, check:vi-mock-specifiers -> exit 0
  • node scripts/check-changeset-presence.mjs -> 3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)
  • Lint, narrowed and declared: eslint . inside packages/app-shell -> 988 files, 0 errors, and the four object-fields-io* files contribute 0 warnings (count read from --format json, universe read from the root config's own files: **/*.{ts,tsx} + ignores). The narrowing excludes nothing relevant: the root eslint.config.js enables no type-aware linting (no project / projectService), so this diff cannot move a verdict on an untouched file. CI runs the full farm regardless.

Every exit code was captured before any pipe; each verdict above is quoted from the gate's own line.

Scope

List unification is deliberately OUT — it spans plugin-designer/src/MetadataFieldsPage.tsx, which #6489 owns in flight. Filed as #6527 (Blocked-by #6489) with the measurement showing the three lists are not simply nested, so a naive union would regress. One more finding filed rather than folded in: #6528.


Generated by Claude Code

…ys on read
`previews/object-fields-io.ts` is the single read door for `draft.fields` across
the object designer, and `writeFields` writes each def back verbatim. Its strip
set named one key (`indexed`) while `FieldSchema` refuses five by name, so a
stored field carrying a sibling round-tripped straight back out to
`PUT /api/v1/meta/object/:name`. Measured on the installed `@objectstack/spec`
17.2.0, through the whole document that endpoint validates, the refusal lands at
`["fields", <name>]` as `unrecognized_keys` — the hard 422 `INVALID_METADATA`
that blocks every later save of the object, with the control that wrote the key
retired and no UI path left to clear it. Same shape objectui#4644 closed in this
file for `indexed`.
Both added keys were verified reachable rather than assumed: `referenceTo` was
emitted by both designer writers until objectui#6041, and `isSystem` was a
declared server-field key the designer read back until objectui#6044. The spec
spellings `reference` and `system` are separate accepted keys and are not
stripped, so nothing the server would store is lost.
Two keys `FieldSchema` also refuses stay out of the list, each for its own
measured reason, both recorded on the tombstone: `formula`, because
`ObjectFieldInspector` migrates it through a linting CEL editor and stripping
empties that editor (measured red pin) — the trade objectui#6043 explicitly
preserved; and `sortOrder`, because no writer on this tree ever populated a
field-level one, so a strip would be dead code that reads like a measurement.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
@github-actions

Copy link
Copy Markdown
Contributor

✅ Console Performance Budget

MetricValueBudget
Eager closure (gzip, 52 chunks)3234.1 KB3266.6 KB
Main entry chunk (gzip)157.4 KB350 KB
Entry fileindex-BXzb7c8X.js
StatusPASS

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

PackageSizeGzipped
app-shell (consoleActionDispatch.js)0.20KB0.19KB
app-shell (index.js)11.32KB4.29KB
app-shell (runtime-config.js)18.10KB6.51KB
app-shell (types.js)0.01KB0.04KB
app-shell (urlParams.js)10.06KB3.86KB
auth (ActiveOrganizationStorage.js)25.05KB9.16KB
auth (AuthContext.js)0.31KB0.24KB
auth (AuthGuard.js)2.07KB1.00KB
auth (AuthProvider.js)40.18KB10.59KB
auth (AuthShell.js)3.49KB1.40KB
auth (ForgotPasswordForm.js)12.21KB3.45KB
auth (LoginForm.js)18.15KB5.39KB
auth (PreviewBanner.js)0.90KB0.50KB
auth (RegisterForm.js)6.65KB2.22KB
auth (SocialSignInButtons.js)9.61KB3.89KB
auth (UserMenu.js)3.41KB1.23KB
auth (auth-gate-events.js)1.29KB0.66KB
auth (authStyles.js)5.04KB1.72KB
auth (createAuthClient.js)40.21KB10.80KB
auth (createAuthenticatedFetch.js)8.46KB3.43KB
auth (index.js)3.19KB1.44KB
auth (invitation-status.js)1.22KB0.70KB
auth (org-roles.js)6.66KB2.78KB
auth (phone-identifier.js)1.11KB0.66KB
auth (types.js)0.59KB0.35KB
auth (useAuth.js)5.30KB1.02KB
auth (useWorkspaceAdminStatus.js)5.13KB2.35KB
collaboration (CommentThread.js)26.08KB7.56KB
collaboration (LiveCursors.js)3.17KB1.27KB
collaboration (PresenceAvatars.js)6.49KB2.64KB
collaboration (PresenceProvider.js)2.79KB1.13KB
collaboration (index.js)1.68KB0.73KB
collaboration (useCollaborationTranslation.js)6.05KB2.52KB
collaboration (useCommentSearch.js)1.98KB0.88KB
collaboration (useConflictResolution.js)7.75KB1.86KB
collaboration (useMentionNotifications.js)1.81KB0.68KB
collaboration (usePresence.js)6.33KB1.84KB
collaboration (useRealtimeSubscription.js)7.91KB2.01KB
components (index.js)506.01KB114.64KB
core (index.js)5.30KB2.13KB
create-plugin (index.js)10.08KB3.26KB
data-objectstack (index.js)173.10KB47.96KB
fields (index.js)238.89KB60.02KB
i18n (LocalizationContext.js)1.76KB0.96KB
i18n (currency.js)1.22KB0.64KB
i18n (fallbackInterpolation.js)6.25KB2.77KB
i18n (i18n.js)4.28KB1.75KB
i18n (index.js)3.44KB1.39KB
i18n (pickLocalized.js)7.62KB3.26KB
i18n (provider.js)26.89KB9.04KB
i18n (useDisplayLocale.js)2.85KB1.45KB
i18n (useObjectLabel.js)33.40KB8.71KB
i18n (useSafeTranslation.js)5.60KB2.33KB
layout (index.js)38.95KB10.97KB
mobile (MobileProvider.js)0.92KB0.49KB
mobile (ResponsiveContainer.js)0.94KB0.38KB
mobile (breakpoints.js)1.51KB0.70KB
mobile (createOfflineDataSource.js)5.61KB1.75KB
mobile (index.js)1.55KB0.62KB
mobile (offlineQueue.js)3.91KB1.35KB
mobile (pwa.js)0.97KB0.49KB
mobile (serviceWorker.js)1.48KB0.62KB
mobile (serviceWorkerSource.js)3.41KB1.48KB
mobile (useBreakpoint.js)1.54KB0.65KB
mobile (useGesture.js)6.96KB1.98KB
mobile (useOfflineSync.js)1.99KB0.72KB
mobile (usePullToRefresh.js)2.53KB0.85KB
mobile (useResponsive.js)0.72KB0.42KB
mobile (useResponsiveConfig.js)1.37KB0.63KB
mobile (useSpecGesture.js)4.32KB1.64KB
mobile (useTouchTarget.js)1.01KB0.54KB
permissions (MePermissionsProvider.js)9.53KB3.38KB
permissions (PermissionContext.js)0.31KB0.25KB
permissions (PermissionGuard.js)0.89KB0.45KB
permissions (PermissionProvider.js)4.64KB1.50KB
permissions (evaluator.js)5.12KB1.74KB
permissions (index.js)0.93KB0.41KB
permissions (store.js)0.91KB0.42KB
permissions (useFieldPermissions.js)1.28KB0.53KB
permissions (usePermissions.js)1.93KB0.88KB
plugin-ai (index.js)15.75KB3.80KB
plugin-calendar (index.js)46.91KB12.92KB
plugin-charts (index.js)64.66KB18.32KB
plugin-chatbot (index.js)188.60KB44.82KB
plugin-dashboard (index.js)133.48KB34.49KB
plugin-designer (index.js)212.76KB43.14KB
plugin-detail (index.js)245.29KB62.39KB
plugin-editor (index.js)2.46KB1.10KB
plugin-form (index.js)131.78KB32.19KB
plugin-gantt (index.js)165.16KB40.33KB
plugin-grid (index.js)201.66KB54.57KB
plugin-kanban (index.js)53.16KB14.65KB
plugin-list (index.js)112.74KB27.50KB
plugin-map (index.js)20.09KB6.62KB
plugin-markdown (index.js)13.72KB4.69KB
plugin-report (index.js)43.51KB11.94KB
plugin-timeline (index.js)26.72KB7.71KB
plugin-tree (index.js)9.26KB3.13KB
plugin-view (index.js)84.85KB20.79KB
providers (DataSourceProvider.js)0.75KB0.39KB
providers (MetadataProvider.js)1.37KB0.59KB
providers (ThemeProvider.js)1.90KB0.85KB
providers (UploadProvider.js)11.66KB3.50KB
providers (index.js)0.45KB0.23KB
providers (types.js)0.01KB0.04KB
react-runtime (index.js)5.62KB2.34KB
react (LazyPluginLoader.js)4.47KB1.63KB
react (SchemaRenderer.js)63.21KB21.05KB
react (data-invalidation.js)5.05KB2.08KB
react (index.js)2.44KB1.21KB
react (schema-input.js)2.32KB1.24KB
react (spec-input.js)0.20KB0.18KB
sdui-parser (codegen.js)5.41KB2.34KB
sdui-parser (dashboard-widget-options.js)3.08KB1.30KB
sdui-parser (index.js)4.93KB2.24KB
sdui-parser (input-type.js)2.84KB1.40KB
sdui-parser (parse.js)12.13KB3.65KB
sdui-parser (provenance.js)3.66KB1.82KB
sdui-parser (types.js)0.28KB0.23KB
sdui-parser (validate.js)7.54KB2.63KB
types (ai.js)0.20KB0.17KB
types (api-types.js)0.20KB0.18KB
types (app.js)2.87KB0.99KB
types (base.js)0.20KB0.18KB
types (blocks.js)0.20KB0.18KB
types (complex.js)2.74KB1.41KB
types (crud.js)0.20KB0.18KB
types (dashboard-filter-alias.js)6.23KB2.74KB
types (data-display.js)3.75KB1.85KB
types (data-protocol.js)0.20KB0.19KB
types (data.js)0.20KB0.18KB
types (designer.js)1.85KB0.85KB
types (disclosure.js)0.20KB0.18KB
types (error-code.js)1.54KB0.88KB
types (feedback.js)0.20KB0.18KB
types (field-types.js)0.20KB0.18KB
types (form.js)0.20KB0.18KB
types (http-inflight.js)8.87KB3.73KB
types (http-retry.js)4.32KB2.02KB
types (icon-key-migration.js)4.26KB1.63KB
types (index.js)4.72KB2.24KB
types (layout.js)0.20KB0.18KB
types (managed-by.js)0.19KB0.18KB
types (mobile.js)2.59KB1.31KB
types (navigation.js)0.20KB0.18KB
types (objectql.js)0.20KB0.18KB
types (overlay.js)0.20KB0.18KB
types (permissions.js)0.20KB0.18KB
types (plugin-scope.js)0.20KB0.18KB
types (record-components.js)0.20KB0.19KB
types (record-semantics.js)1.28KB0.67KB
types (registry.js)0.20KB0.18KB
types (reports.js)0.20KB0.18KB
types (spec-report.js)5.05KB1.93KB
types (spec-ui-namespace.js)0.20KB0.19KB
types (system-fields.js)3.33KB1.54KB
types (theme.js)6.28KB2.87KB
types (ui-action.js)3.40KB1.71KB
types (views.js)0.20KB0.18KB
types (widget.js)0.20KB0.18KB

Size Limits

  • ✅ Core packages should be < 50KB gzipped
  • ✅ Component packages should be < 100KB gzipped
  • ⚠️ Plugin packages should be < 150KB gzipped

@os-support-aiClaude

Copy link
Copy Markdown
Collaborator

ACCEPT — objectui#6519 (domain:ui lane, PM review). Reviewed from the tree at 273a7d4d1, not from the report.

Headline: the card named four missing keys and asked for them to be stripped. Measured per key, they gave four different answers — and two of them say do less, not more. The PR delivers two, falsifies one outright, and escalates one rather than implementing it. That is the right outcome and the diff alone could not tell you so.

Why the three-key list is a measurement, not an oversight

A RETIRED_FIELD_KEYS of three where the card said five is exactly what an incomplete job looks like. Verified from the tree, it isn't:

keyverdictevidence
referenceTostripemitted by both writers until #6041
isSystemstripa declared ServerFieldSchema key the designer read back until #6044
sortOrderpremise falsifiedno writer on this tree ever populated a field-level one
formulapremise holds, strip does notstripping destroys authored source

Confirmed at object-fields-io.ts:118['indexed', 'referenceTo', 'isSystem'].

⭐⭐⭐ sortOrder: the tombstone names its own falsifier

#6045 removed it as #4687's shape — a declaration with zero readers and zero writers — not as #6041's rename. It was declared on the UI model and the wire shape and left undefined, so toFieldPayload emitted sortOrder: undefined and JSON.stringify dropped it. No shipped build stored one, so no draft this door reads can carry one.

Adding it anyway would have been dead code that reads like a measurement — the most expensive kind, because the next reader counts four entries and concludes the population was surveyed.

What makes the omission honest rather than a gap is the last line of the tombstone (object-fields-io.ts:96):

Evidence of a stored field-level sortOrder would change this — add it then, not defensively.

A negative result that states the observation which would overturn it is a finding. One that just doesn't mention the key is a hole. Also handled: the object-level sortOrder on ObjectDefinition and the saved-view sortOrder in ObjectView are named and set aside as different concepts living outside fields — the same-name-different-concept trap, caught rather than tripped.

⭐⭐⭐ formula: ablating the FORBIDDEN option in the other direction

The usual ablation proves the banned smaller fix goes red. This one proves the larger fix does — that stripping one more key, the obvious "finish the pattern" move, is the wrong answer:

ObjectFieldInspector.tsx:561 seeds its linting CEL editor from readPredicate(def.expression ?? def.formula), and :565 commits to expression, so the first edit migrates the key — which is precisely what #6043 preserved when it refused a blind rename. Strip formula at the read door and that editor opens empty, and the authored source is gone on the next save.

Measured, not reasoned:

FAIL … > commits edits to `expression` and migrates the legacy `formula` key
expect(element).toHaveValue(record.amount)
Received: ""
Tests 1 failed | 42 passed (43)

Silent data loss wearing the costume of consistency. Leaving it out cost one line and a paragraph; taking it would have cost authors their formulas.

The escalation is correctly placed, and I am not adjudicating it

#6526 carries the live half: a draft holding formula still round-trips to a hard 422, and the two ways out trade against each other. Option A destroys authored data to unblock a save. This lane's standing rule is that a fix which destroys data is human floor even when the card is graded Bug and the defect is undisputed — so #6526 belongs in the decision box, and the dev put it there with both options and a four-axis recommendation (B) rather than picking. The recommendation reads sound to me and I am explicitly not ruling on it. One thing worth carrying into that decision, from the dev's own note: if A is chosen, the tombstone and the inspector pin must change in the same PR — they currently record the opposite.

Instrument hygiene

  • Reverse verification asserted on the emitted body, not a helper's return value: the new cases build the object document PUT validates and parse it with the real ObjectSchema; each un-round-tripped draft is refused at path fields.amount with unrecognized_keys — the 422's own shape — and the round-tripped body parses green, with a control proving the document is otherwise accepted. A pin on the helper's output would have passed while the wire stayed broken.
  • Mutations proven on disk by counting the target text (four-key literal 1→0, three-key 0→1 at each step), not by an editor's exit code. No package dist/ between mutation and assertion; the only prebuilt artifact in the loop is @objectstack/spec, the unmutated oracle.
  • One of the dev's own fixtures needed triage rather than a rule change: the array-shape case in spec-keys carried formula, which now correctly survives — so it was swapped to referenceTo. Left alone it would have become a vacuous assertion quietly passing for the wrong reason.
  • The repo's vitest guard (objectui#3378) rejected the first invocation; booked NOT MEASURED and re-run from the repo root. Fourth dev today — the guard works, and the repeat rate is a devx datum.
  • Four TS2307s in a fresh worktree classified as prerequisite-not-met, not a broken import, and resolved by building the dependency closure. Reporting your own self-inflicted false alarm is what makes the other numbers credible.

Fence

Nothing under packages/plugin-designer/ — so #6489 kept sole ownership of MetadataFieldsPage.tsx for its whole run, and list unification is out and filed (#6527). The fence I set held exactly.

CI at review time: 29 checks, zero failed, 6 running. behind is not a call to action here — the merge queue handles it. Landing on green.


Generated by Claude Code

@os-support-ai
os-support-ai marked this pull request as ready for review August 26, 2026 09:20
@os-support-ai
os-support-ai added this pull request to the merge queueAug 26, 2026
Merged via the queue into main with commit c18acb0Aug 26, 2026
30 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-6519-retired-field-key-strip branch August 26, 2026 09:33
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding(app-shell): object-fields-io strips only indexed on read, so the four other retired designer keys round-trip straight back out to a 422

2 participants

@os-support-ai@claude