Commit da35453
fix(studio): object writers seeded from a served read drop a picklist-bound field's served options (objectui#11692) (#11706)
Fixes #11692
Clause-②: yes
Object writers outside the two objectui#10202 designers seed their PUT
from a served object read. The runtime serves a picklist-bound field
with `picklist` and the `options` it resolved from the list, and the
authoring door refuses that pair for the whole object (`422
INVALID_METADATA` at `fields.FIELD.options`). Each writer the card lists
now either drops those served options before its PUT, or is shown here
not to seed from a served read. A fifth writer of the same class,
`EmbeddedItemEditor`, was found during the work. The seat added it to
the claim's file surface (comment 6012529702), and it is fixed the same
way.
Dispatched work: claim 6011332561 on objectui#11692, PM session
`https://claude.ai/code/session_01FngvPpdrnhHMdHHq6vwwju`.
## What changes
**The helper moves to `@object-ui/data-objectstack` (new export, the
reason for `Clause-②: yes`).**
- `dropServedPicklistOptions` moves from
`app-shell/src/views/metadata-admin/picklist-binding.ts` to
`data-objectstack/src/picklist-binding.ts` (a `git mv`, with its test)
and is exported from the package barrel.
- Why it moves: `@object-ui/plugin-designer` does not depend on
`@object-ui/app-shell`, and `app-shell` lists `plugin-designer` as a
peer, so the two Setup pages could not import it where it lived. Both
packages already depend on `data-objectstack`. That package also owns
the client that serves the read and carries the write, the same reason
`extractDraftBody` lives there.
- The two existing callers, `builtinComponents.tsx` (the metadata-admin
`object` resource's `fromDraft`) and `StudioDesignSurface.tsx`, now
import it from `@object-ui/data-objectstack`. No copy is left in
`app-shell`, which never exported it.
- Built declaration, read from
`packages/data-objectstack/dist/index.d.ts` after `turbo run build`:
`declare function dropServedPicklistOptions` with type parameter `T
extends Record of string to unknown`, taking `body: T` and returning
`T`. It appears in the barrel's export list beside `extractDraftBody`.
The type parameter is spelled out in words here because GitHub strips
angle brackets from PR bodies.
- ⛔ It is not applied at a door. `MetadataClient.save` is unchanged,
which leaves option B ruled out as triage directed. The barrel comment
and the module docblock both say the export is for WRITERS. A door
cannot tell a served copy from an authored pair, and the authored pair
must stay the server's loud refusal.
**Per writer (card "Done when"):**
| Writer | Seeds from a served read? | Now |
|---|---|---|
| `PackageOwdOverviewPanel` `doSave` | yes: `getDraft` body, else
`layered.effective` | `dropServedPicklistOptions(next)` before
`client.save` |
| `MetadataFieldsPage` `handleFieldsChange` | yes:
`client.get('object')`, and `carryOver` / `carryPreservedField` carry
every per-field key back | `dropServedPicklistOptions(mergedObject)`.
The page has no options editor, so a bound field's `options` can only be
the served copy |
| `MetadataObjectsPage` `handleObjectsChange` | yes:
`client.list('object')` spread as the merge base |
`dropServedPicklistOptions(merged)` |
| `EmbeddedItemEditor` `doSave` (opened from an object's fields, indexes
or validations) | yes: it re-reads `client.layered(parentType,
parentName).effective`, splices the edited item in, and PUTs the whole
parent | an `object` parent goes through
`dropServedPicklistOptions(updated)`. Other parent types are sent
exactly as before |
| `MetadataService.saveFields` | yes: `client.meta.getItem('object')`
and the per-field carry-over |
`dropServedPicklistOptions(updatedObject)`. `DesignerFieldDefinition`
declares no `picklist`, so this method can neither author nor remove a
binding |
| `MetadataService.saveObject` | **no**: it reads nothing, and builds
from the caller's `obj` and `existingFields` | unchanged, with the
reason in its docblock |
| `MetadataService.saveMetadataItem` | **no**: it reads nothing, and
`data` is the caller's | unchanged, with the reason in its docblock |
The list read resolving the binding is a source reading of objectstack,
not a live measurement. objectstack's `metadata-protocol`
`foldObjectExtendersFromRegistry` docblock says the list read "reads
`registry.listItems`, whose object branch resolves through the same
fold", and `SchemaRegistry.foldExtendersOntoDefinition` ends in
`resolvePicklistFields`.
Triage's correction stands: `runtime-metadata-persistence.ts` saves no
object, so it is untouched.
## Pins (a 422-refusing transport, as objectui#10202's pins are)
Each new test drives a real `MetadataClient`, or for `MetadataService` a
real `ObjectStackAdapter` and SDK. The transport double answers a PUT
whose fields carry `picklist` beside `options` with the door's `422
INVALID_METADATA` envelope, so every claim is read off the wire:
- `PackageOwdOverviewPanel.picklistServedOptions-11692.test.tsx`: an OWD
change on an object with a bound field, seeded from `layered.effective`
and from a served draft. The door accepts the body, the bound field goes
out as `picklist` only, the inline select and the text field go out
byte-identical, and the OWD edit lands.
- `MetadataFieldsPage.picklistServedOptions-11692.test.tsx`: relabel an
unrelated field. Both the designable bound `select` and a
carried-through bound `multiselect` leave without `options`, and the
inline select keeps its options.
- `MetadataObjectsPage.picklistServedOptions-11692.test.tsx`: relabel
the object. Same assertions.
- `EmbeddedItemEditor.picklistServedOptions-11692.test.tsx`: the editor
is mounted over a served object with a bound `tier`. Two cases: saving
another item (`amount`), and an untouched save of the bound field
itself. In both, the door accepts the parent, `tier` goes out as
`picklist` only, and the inline select goes out byte-identical.
- `MetadataService.picklistServedOptions-11692.test.ts`:
- `saveFields` with a field list built from the served read is accepted,
and the bound field has no `options`.
- `saveObject` and `saveMetadataItem` issue no read, and an authored
pair still reaches the door and is refused, asserted on the SDK error's
`code: 'INVALID_METADATA'` and `httpStatus: 422`.
- The moved `picklist-binding.test.ts` is unchanged apart from its
header. It still reads the contract off the installed
`@objectstack/spec`.
**Ablation.** Each fix was committed first (`51bb8bb` for the four
writers, `32b8c2e` for `EmbeddedItemEditor`). Each writer's helper call
was then removed through objectstack's `scripts/ablation-replace.mjs`,
which wraps the run, verifies that the mutation landed on disk, and
restores from `HEAD`. Every pin went red for the right reason, and every
restore was proven by blob hash equal to HEAD and an empty `git diff
HEAD`.
| Mutation | Result | Failure message |
|---|---|---|
| OWD `dropServedPicklistOptions(next)` to `next` | 2 failed of 2 |
`refused at fields.tier.options: expected 422 to be 200` |
| fields page `dropServedPicklistOptions(mergedObject)` to
`mergedObject` | 1 failed of 1 | `refused at fields.tier.options,
fields.regions.options` |
| objects page `dropServedPicklistOptions(merged)` to `merged` | 1
failed of 1 | `refused at fields.tier.options` |
| `saveFields` `dropServedPicklistOptions(updatedObject)` to
`updatedObject` | 1 failed, 2 passed of 3 | the SDK's `failed spec
validation` throw |
| `EmbeddedItemEditor` `dropServedPicklistOptions(updated)` to `updated`
(at `32b8c2e`) | 2 failed of 2 | `refused at fields.tier.options:
expected 422 to be 200` |
The `saveObject` and `saveMetadataItem` pins stay green under that last
mutation, as they should. The first attempt at the last three ablations
was REFUSED by the tool and ran no test, because the replacement was a
substring of the anchor. They were re-anchored on the whole `save(...)`
line and re-run. The table shows the re-run.
No `dist` build sits in the ablation path. The writers are imported from
`src` by relative path, and the root vitest config aliases
`@object-ui/data-objectstack` to its `src`.
## Gates
**Patch round (fifth writer), at branch head `32b8c2e`.** This head
includes the merge of `main` at `3409fe8`, which is a merge commit, not
a rebase.
| Gate | Exit | Verdict line |
|---|---|---|
| `turbo run build --filter=@object-ui/app-shell^... --concurrency=2` |
0 | `Tasks: 28 successful, 28 total` |
| `pnpm --filter @object-ui/app-shell run type-check` | 0 | |
| `pnpm --filter @object-ui/app-shell run lint` | 0 | 0 errors. The
warning count is unchanged from `bc90d2f`, and the new test has no hit |
| `vitest run` of the 6 pin files plus the 3 existing
`EmbeddedItemEditor` suites | 0 | `Test Files 9 passed (9)`, `Tests 39
passed (39)` |
| `check:new-line-citations` / `check:control-bytes` /
`check:changeset-claims` / `check:pending-changeset-literals` /
`check:metadata-write-doors` | all 0 | `0 new citation(s)` and `3 can
carry an object document, 3 reach assertObjectMetadataWritable` |
| `check-changeset-no-major` / `check-changeset-presence` /
`check:self-import` / `check:phantom-deps` /
`check-vi-mock-{specifiers,inherit,override-shape}` /
`check:test-path-roots` | all 0 | `15 source file(s) of 3 released
package(s) changed, and this change declares 1 changeset(s)` |
**Round 1, at branch head `bc90d2f`.** These readings were not re-taken
in the patch round. All at `bc90d2f` unless stated. Heavy runs went
through `os-verify-lock.sh`. Every exit code was captured before any
pipe.
| Gate | Exit | Verdict line |
|---|---|---|
| `turbo run build --filter=@object-ui/app-shell^...
--filter=@object-ui/plugin-designer^... --concurrency=2` (at `51bb8bb`,
whose `src` is identical to `bc90d2f`) | 0 | `Tasks: 28 successful, 28
total` |
| `pnpm --filter @object-ui/data-objectstack run type-check` | 0 | |
| `pnpm --filter @object-ui/plugin-designer run type-check` (`tsc
--noEmit && tsc -p tsconfig.test.json`) | 0 | the new tests are program
inputs (`--listFiles`) |
| `pnpm --filter @object-ui/app-shell run type-check` (same two legs) |
0 | the new tests are program inputs (`--listFiles`) |
| `pnpm --filter @object-ui/{data-objectstack,plugin-designer,app-shell}
run lint` | 0 / 0 / 0 | 0 errors. No warning on a line this PR adds |
| `vitest run` of the 5 new or moved test files | 0 | `Test Files 5
passed (5)`, `Tests 13 passed (13)` |
| `vitest run packages/data-objectstack/ packages/plugin-designer/` plus
126 `app-shell` files (see below) | 0 | `Test Files 247 passed (247)`,
`Tests 2342 passed (2342)` |
| `check:self-import` / `check:phantom-deps` / `check:unused-deps` | 0 /
0 / 0 | |
| `check:readme-exports` | 1, a precondition (see below) | the new
import is judged `real` (`--list`) |
| README snippet, compiled `--strict` against the built `dist` | 0 |
negative control (a misspelt name): TS2724, exit 2 |
| `check:new-line-citations` | 0 | `0 new citation(s)` |
| `check:control-bytes` | 0 | `OK` |
| `check:metadata-write-doors` | 0 | `3 can carry an object document, 3
reach assertObjectMetadataWritable` |
| `check:changeset-claims` / `check:pending-changeset-literals` | 0 / 0
| `No pending changeset names a file this change touches.` |
| `check-changeset-no-major` / `check-changeset-fixed` /
`check-changeset-presence` | 0 / 0 / 0 | `13 source file(s) of 3
released package(s) changed, and this change declares 1 changeset(s)` |
| `check:unreferenced-sources` / `check:test-path-roots` /
`check-vi-mock-{specifiers,inherit,override-shape}` /
`check:esm-specifiers` / `check:doc-fences` /
`check:designer-field-key-parity` / `check-type-check-coverage` /
`check-lint-coverage` / `check:spec-symbols` /
`check:component-surface-parity` (report-only) | all 0 | |
Notes on the table:
- **Narrowed test run, declared.** `app-shell` has 1025 test files. A
full run of all three packages was started and held the shared lock for
29 minutes without finishing, with other seats queued behind it, so I
stopped it (my own PID). I re-ran a narrowed set instead: all of
`data-objectstack` and `plugin-designer`, and in `app-shell` every test
under `src/services/` and `src/views/studio-design/`, every test that
names `StudioDesignSurface`, `builtinComponents`,
`PackageOwdOverviewPanel`, `MetadataService`, `picklist-binding`,
`dropServedPicklistOptions` or `register-builtins`, and the
objectui#10202 suites. That set is derived from direct importers. It is
blind to a test that reaches a touched module through a third module, so
the full `app-shell` suite is left to CI. `app-shell`'s barrel surface
is byte-unchanged. `data-objectstack` gains one export and loses none.
- **`check:readme-exports` exit 1 is not a measurement of this change.**
It is the gate's "type entry not on disk" refusal for 8 packages outside
this build's closure (`plugin-gantt`, `plugin-map`, `plugin-markdown`,
`plugin-timeline` and others). With `--list` it exits 2 (PRECONDITION
NOT MET) and judges `packages/data-objectstack/README.md`'s
`dropServedPicklistOptions` import `real`.
- **NOT MEASURED: `check:doc-snippets`, reason: its build closure is
every package.** The one new README block was compiled `--strict`
against the built `data-objectstack` dist instead, with the negative
control shown in the table.
- **NOT MEASURED: `check:eager-closure`, reason: it needs a full console
build.** See Acceptance notes.
## Acceptance notes
- **The fifth writer, `EmbeddedItemEditor`, is fixed here** after the
claim amendment (comment 6012529702). Its wrap covers the edited item
too, the same rule the four writers and the Studio data page apply:
every field that names a picklist leaves without `options`. If the
drawer's generic field form offers `options` on a bound field, an edit
made there is dropped while the binding stays, because the editor cannot
tell the served copy from an edit. Unbinding a field stays the job of
the select-field editor's "Options from" picker (objectui#10202).
- `MetadataService` has no in-repo caller of `saveObject`, `saveFields`
or `saveMetadataItem`. It is public API through `useMetadataService`.
The `saveFields` fix and the two pins of "no read, the pair still
refused" are therefore measured on the method, not on a screen.
- `check:eager-closure` was not run locally (it needs a full console
build). The helper moved into a package that
`apps/console/src/dataSource.ts` already imports statically, so no new
module joins the eager graph. That claim is read from source, not
measured.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FngvPpdrnhHMdHHq6vwwju)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent de96f3d commit da35453
17 files changed
Lines changed: 942 additions & 21 deletions
File tree
- .changeset
- packages
- app-shell/src
- services
- views
- metadata-admin
- studio-design
- data-objectstack
- src
- plugin-designer/src
| 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 | + | |
Lines changed: 156 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
| |||
704 | 705 | | |
705 | 706 | | |
706 | 707 | | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
707 | 716 | | |
708 | 717 | | |
709 | 718 | | |
| |||
804 | 813 | | |
805 | 814 | | |
806 | 815 | | |
| 816 | + | |
| 817 | + | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
| 821 | + | |
| 822 | + | |
| 823 | + | |
807 | 824 | | |
808 | 825 | | |
809 | 826 | | |
| |||
894 | 911 | | |
895 | 912 | | |
896 | 913 | | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
| 921 | + | |
| 922 | + | |
| 923 | + | |
| 924 | + | |
| 925 | + | |
| 926 | + | |
| 927 | + | |
| 928 | + | |
| 929 | + | |
897 | 930 | | |
898 | 931 | | |
899 | 932 | | |
| |||
944 | 977 | | |
945 | 978 | | |
946 | 979 | | |
947 | | - | |
| 980 | + | |
948 | 981 | | |
949 | 982 | | |
950 | 983 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
29 | 32 | | |
30 | 33 | | |
31 | 34 | | |
| |||
0 commit comments