Repository navigation
fix(plugin-form,fields): a form neither submits nor offers a field the caller may read but not edit (objectui#10120) - #10161
Conversation
…e caller may read but not edit The platform refuses a write to a field the caller's permission set marks `editable: false`, and cannot tell a round-trip of the value it just served from an attempted write. A form echoing an UNCHANGED value back is therefore a 403 for the whole save, even when the user touched only a field they are allowed to edit. The verdict that answers "may this caller edit this field" already existed and one path already used it: `checkField(object, field, 'write')` in `@object-ui/permissions`. What had drifted is that each form container spelled its own COPY of the outbound strip and of the render pass — the same defect class as the server-owned roster one layer down — and the third container of the family carried neither half. Measured on one record with one permission set: the simple form and the modal withheld the refused column while the drawer sent it, and the drawer rendered it as a live input while the other two rendered it disabled. - The field-level verdict now reaches the ONE outbound filter as a predicate (`sanitizeFormData`'s `canEdit`), adapted from that resolver by `fieldWriteGate`, instead of as a strip loop after each container's call. - The render pass is likewise one function, `applyFieldPermissions`, for all three containers. `DrawerForm` gains both halves; the other two produce the payloads and the markup they produced before. - Both halves stay fail-open with no permission provider mounted. - A lookup's selected chip no longer offers its remove control when the field is disabled — the one affordance that could still change the value while the trigger and the browse button were already gated. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
|
changeset-claim-re-read
|
…rm-omits-fls-denied-fields
✅ 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
|
1 similar comment
✅ 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
|
ACCEPT — PR #10161, card objectui#10120.
|
| container | on origin/main |
|---|---|
ObjectForm |
perms.checkField(…, 'read') / 'write' at two sites |
ModalForm |
the same calls, under // FLS gate: drop non-readable fields, disable non-editable ones. — and a second strip at the payload, under // FLS defence-in-depth: strip non-editable fields from payload. |
DrawerForm |
⛔ neither. Zero checkField, no FLS comment — only filterSystemFields and sanitizeFormData |
⭐ The same grep finding the call in two of three files is what makes the third's zero a reading and not a dead instrument.
⇒ 「one family with three answers: each container spelled its own COPY of the outbound strip and of the render pass, and the third carried neither」 is exact.
⭐ And it explains the card rather than contradicting it. The card's repro opens the form from a related list's row menu 「编辑」 — which is a drawer. The reporter was on the one container that carried no copy. A card whose premise reads false against two containers and true against the third was not wrong; it was under-scoped, and the dev found the scope.
The second half the card asked for, which was fully live
A LookupField chip offered a live remove control while the trigger and browse button were already disabled. ⇒ the widget was half-gated: two of its three affordances respected the refusal and the one that mutates did not. ⭐ That is the worst arrangement of the three, because the disabled trigger tells the user the field is locked and the ✕ then says otherwise.
The dispatch's central demand, met
⭐ 「Find the existing answer and reuse it; ⛔ do not write a second implementation.」 The answer is checkField(object, field, 'write') in @object-ui/permissions, resolved by MePermissionsProvider from the server's /me/permissions envelope — ⛔ no rung of it re-derived. fieldWriteGate adapts that one verdict into a predicate the one outbound filter takes; applyFieldPermissions is the single render pass all three containers now share.
Fail-open with no permission provider mounted is preserved unchanged — ⛔ correctly not moved by this card.
The ablation, and the one honest gap in it
Mutated: 7 failed | 9 passed, and the seven are exactly the three container payload rows, the two filter rows and the two chip rows. Restored: 16 passed, both files' blobs equal to HEAD by git hash-object with an empty git diff HEAD, under a trap with absolute paths. Mutation proved by anchor counts on disk (a guard line 1 → 0; two gated chip sites collapsed onto one constant), ⛔ not by an editor's exit code. No dist is involved — the root vitest config aliases every @object-ui specifier to that package's src, so the mutation is what the tests load.
⭐ The gap is declared rather than hidden: the two render-gate rows stay green under this ablation because it does not touch applyFieldPermissions. Their before-state is the direct pre-fix measurement on unmodified origin/main quoted above — which this seat has now independently confirmed. An ablation that names what it does not cover is worth more than one that quietly covers less than it claims.
Clause-② — no, re-derived here and ⛔ not taken
The barrel is src/index.tsx, 30 named export lines (the lit control), ⛔ zero export * forms, and it names neither sanitize nor fieldWriteGate; the nine-file diff does not touch it. The dev's independent reading used a positive control too — on the built dist/index.d.ts, sanitizeFormData 0, fieldWriteGate/applyFieldPermissions 0, ObjectForm 6. Two instruments, one answer. --pair 10161 exits 0; check-governed-merges.mjs --pr objectstack-ai/objectui#10161 exits 0.
⭐ The dev also declined to post the Clause-②-correction: itself, on the ground that the carrier is the seat's to post and would have been a fifth API write outside its declared budget. ⛔ That is correct and it is recorded here: the carrier is the seat's act, and the seat posted it.
A declared narrowing this seat accepts
The single-package vite build used for the Clause-② measurement ran outside the shared verify lock, declared: the lock was held by the sibling objectui#10119 lane through two consecutive nine-minute queue timeouts (exit 99, never acquired, holder running the app-shell suite). A single package's vite build is check-gate weight, ⛔ not a whole-repo build.
Findings routed
objectui#10163 — the master-detail line-items grid consults no field permission, so an FLS-denied column renders as a live editable cell one container outside this fix. Filed with the relay clearly marked (⛔ measured by neither the dev on its branch nor this seat) and with its step 1 being a lit-control reproduction.
⛔ Not filed, agreed, each with its successor named: the outer schema gate no-oping on a bare-string fields entry (harmless — every container re-applies the pass downstream); and MasterDetailField.tsx's unguarded remove control, which carries a tombstone comment declaring itself unreachable and is a declared orphan awaiting a maintainer decision on removing a public export — ⛔ touching it would reopen that decision.
Generated by Claude Code
…default and drawer arms (objectui#9849 step 1) (objectstack-ai#10236) Part of objectstack-ai#9849 Clause-②: no Step (1) of the two the director ruling orders on this card (comment `5749155880`, letter E, maintainer 「同意」): 「the three `collapsed` / `collapsible` resolutions converge to the declaration-based one objectui#9780 established, before the gate rule lands on it.」 Step (2) (E in `projectSectionDivider`, deleting `SectionDividerGate`, re-pinning `drawerFormSectionDescription-9834` row 5, the diagnostic pin) is ⛔ not in this PR and stays on the card. That is why the first line says `Part of`. ## What changed - `fieldGroups.ts` gains `resolveSectionCollapse`, beside `projectSectionDivider`. It is module-internal: the package barrel `src/index.tsx` names `fieldGroups` zero times, and `package.json` `exports` declares only `"."`. It is the ONE resolution of the authored pair, and it follows objectui#9780 letter A: - `collapsed` implies `collapsible`, read off the declaration and never off the live state. - `collapsible: false` with `collapsed: true` resolves to collapsed, with the control present. - `collapsible` alone is open, with the control. - A section declaring neither member is untouched. - A section is only collapsed while it is collapsible, and it is only collapsible when the row the arm draws can carry the control (`hostsControl`). This states the default arm's 「an untitled bucket is never collapsible」 once, for every arm. - `ObjectForm.tsx` (grouped layout, explicit and derived sections share one loop): the inline objectui#9780 expression moves into the helper. Behaviour is unchanged (see the table). - `DrawerForm.tsx`, both pushes, now call the same helper: - The explicit push was still the objectui#9780 trap: it read `collapsible` alone for the control and `collapsed` unconditionally for the state. - The derived push gated the state on `collapsible`, which is a third spelling. - The unseeded `useState` replaces a seed that read `collapsed` a second time. An untoggled section falls back to its declaration inside the helper, as in ObjectForm. - `.changeset/9849-collapse-resolution-converge.md`: `patch` for `@object-ui/plugin-form`. ⛔ No new accepted key or value, no new package export, no new prop. `DrawerFormSectionConfig` and `ModalFormSectionConfig` are unchanged, and no section map in `ObjectForm.tsx` copies a new key. ## Premise, re-measured on `origin/main` `62597c5` by printing every `collaps` line in the four files - **ObjectForm grouped layout, one site** (the objectui#9780 resolution): `isCollapsible = Boolean(section.collapsible) || Boolean(section.collapsed)`, the state `collapsedSections[key] ?? section.collapsed`, and fields hidden only on `label && isCollapsed`. - **DrawerForm explicit push**: `collapsible: section.collapsible`, `onToggle` only for `section.collapsible`, and the state `collapsedSections[key] ?? section.collapsed` read unconditionally, which hides the fields. A `useState` seed also read `s.collapsed`. - **DrawerForm derived push**: the state read only when `section.collapsible`, the control only for `section.collapsible`. - **ModalForm: zero `collaps` lines.** It has no resolution of its own. Neither route reads either member, `ModalFormSectionConfig` declares neither, and `ObjectForm`'s modal map copies neither. So the modal draws every field and no control, whatever the section declares. - `fieldGroups.ts` at base only maps the `@objectstack/spec` `collapse` enum onto the pair (`deriveFieldGroupSections`) and copies a resolved pair onto the row (`projectSectionDivider`). Neither step is a resolution. ⇒ The code has **three resolutions, as the ruling says**. objectui#10172, objectstack-ai#10161 and objectstack-ai#10141 did not move them. But there are only **two in behaviour**: - The derived drawer push agrees with objectui#9780 on every input an author can write. `deriveFieldGroupLayout` never emits `collapsed` without `collapsible` (the deprecated `collapsed: true` alone maps to `'collapsed'`, which maps to both), and every keyed group gets a heading (label defaults to the key). - Ablation leg 2 below measures this: putting the derived push back on its old resolution reddens nothing. - The live trap was the explicit drawer push only. ## Before / after, per route Read by `__tests__/collapseResolution-9849.test.tsx` through a real renderer. "Before" means the same pin file run against the base source: `Tests 9 failed | 21 passed (30)`. Every failure was on a drawer explicit route or a drawer trap row, and each failed for the right reason (0 controls while the fields were hidden). | route | `collapsed` alone | `collapsible: false` + `collapsed` | `collapsible` alone | neither | no heading, no blurb, `collapsed` | no heading, blurb, `collapsed` alone | | --- | --- | --- | --- | --- | --- | --- | | ObjectForm default, explicit `sections` | closed + control → **same** | closed + control → **same** | open + control → same | open, no control → same | open → **same** | open, no control (blurb-only row, objectui#9835 B) → same, not re-pinned here (`objectFormSectionMembers-8071` row 8 owns it) | | ObjectForm default, derived `fieldGroups` | closed + control → same (both `collapse: 'collapsed'` and deprecated `collapsed: true`) | n/a | open + control → same | open, no control → same | not author-reachable | not author-reachable | | ObjectForm `formType: 'drawer'` (drawer map) | ⛔ closed, **0 controls** → closed + control | ⛔ closed, **0 controls** → closed + control | same | same | ⛔ fields hidden, **nothing on the page** → open | see next row | | DrawerForm explicit, direct | ⛔ closed, **0 controls** → closed + control | ⛔ closed, **0 controls** → closed + control | same | same | ⛔ fields hidden, **nothing on the page** → open | ⛔ blurb drawn, fields hidden, **0 controls** → closed, control on the blurb row | | DrawerForm derived `fieldGroups` | closed + control → same | n/a | same | same | not author-reachable | not author-reachable | | ModalForm explicit, ObjectForm `formType: 'modal'`, ModalForm derived | every field drawn, no control → **same** (a reading, see below) | same | same | same | same | same |⚠️ The last column differs between the default arm (open, no control) and the drawer (a control on the blurb row). That is the **gate** residual (`SectionDividerGate`, step 2), ⛔ not the resolution. The default arm's blurb-only row carries no collapse pair under objectui#9835 letter B. The drawer's explicit push draws a row for every member, and `SectionDivider` renders that row when it has a heading or a blurb. Both answers keep the fields reachable, and that is all the new file asserts there.⚠️ The modal rows are a READING, ⛔ not a ruling. The modal honours neither member, so objectui#9780's one promise (a declared section is never unreachable) holds there trivially. The pin asserts that, and it records "no control" so that step (2)'s 「on every arm」 (ruling E item 1) flips those rows on purpose. Bringing the pair to the modal needs a new member on `ModalFormSectionConfig`, which this step does not add. ## Evidence, all at head `8cb4574b` **Pins.** New `packages/plugin-form/src/__tests__/collapseResolution-9849.test.tsx`, 30 rows: - one block per route that emits a collapse pair (3 explicit, 2 derived), each asking the objectui#9780 questions; - the drawer trap rows; - the modal reading. Every zero is lit: each absent-fields row also reads the heading drawn and the same reader non-empty after a click, and each no-control row is paired on its route with a row where the same reader finds one. - Before, on the base source: `Tests 9 failed | 21 passed (30)`. - After: 30 passed. **Ablation.** Run through `ablation-replace.mjs`: each anchor hit exactly once, each write was verified on disk by blob hash and marker count, and each restore was proven by state (blob equals HEAD, `git diff HEAD` empty) with a wrapper trap on EXIT/INT/TERM. The run covered the new file plus `collapsedImpliesCollapsible-9780`, `objectFormSectionMembers-8071` and `fieldGroupSections.test.tsx` (53 tests). Directions were written down before the run, and all three matched: | leg | mutation (blob before → during) | result | | --- | --- | --- | | 1 | drawer EXPLICIT push back on its old resolution (`d0dc3ee90d5b` → `c0de0a1cc600`, marker 0 → 3 → 0) | `Tests 9 failed \| 44 passed (53)`: exactly the 9 drawer explicit and drawer trap rows. **Every ObjectForm row stays green**, and so do the objectstack-ai#9780 and 8071 files | | 2 | drawer DERIVED push back on its old resolution (`d0dc3ee90d5b` → `4bda9e2214c5`) | `Tests 53 passed (53)`: predicted, because the old derived resolution is behaviourally identical on every author-reachable input | | 3 | the ONE helper loses the implication (`c24e82e8e24c` → `8a713103f65a`) | `Tests 10 failed \| 43 passed (53)`: the `collapsed`-alone and contradiction rows on **all three explicit routes at once**, the drawer blurb row, `collapsedImpliesCollapsible-9780` rows 1 and 3, and `objectFormSectionMembers-8071` row 4. One mutation reddens the default arm and the drawer together, which shows that one path serves both | No `dist/` is involved: the suite imports `plugin-form` from source through the root vitest config, so no dist preflight applies. **Suites and gates** (heavy runs under the shared verify lock, `VERDICT command-exit 0`): - `pnpm exec vitest run packages/plugin-form/` from the repo root: `Test Files 109 passed (109)`, `Tests 1074 passed | 1 skipped (1075)`. - `pnpm --filter @object-ui/plugin-form run type-check` echoed `tsc --noEmit && tsc -p tsconfig.test.json` and exited 0, after building the dependency closure (`--filter '@object-ui/plugin-form^...' build`, exit 0). `tsc -p tsconfig.test.json --listFilesOnly` lists the new pin file, so its types are checked. - Lint, as a DECLARED NARROWING to the 4 touched `.ts`/`.tsx` files: - population: `eslint.config.js` applies to `**/*.{ts,tsx}`; - count: `--format json` reports 4 files, 0 errors; - invariance: the config enables no type-aware linting (no `projectService`, no `parserOptions.project`), so this diff cannot move the verdict of a file it does not touch; - warnings on the edited sources are unchanged against BASE (ObjectForm 61 → 61, DrawerForm 22 → 22, fieldGroups 0 → 0). The new pin's 13 warnings are all `no-explicit-any` schema casts, the sibling pins' pattern; - the repo-wide `pnpm lint` is CI's. - `pnpm check:control-bytes`, `check:new-line-citations` (0 new), `node scripts/check-changeset-presence.mjs` (names the changeset), `check:changeset-claims`, `check:pending-changeset-literals`, `check:test-path-roots`, `check:vi-mock-specifiers` and `check:vi-mock-override-shape` all exited 0. `check-governed-queue-guard --test` over the 5 paths reported NOT GOVERNED. A manual control-byte grep over the 5 files found no match. -⚠️ NOT MEASURED locally: the CI farm, including `Spec Main Shape Gate`, which is red on every PR today on untouched test files (objectui#10229). ⛔ Not addressed here. ## Acceptance notes - **Two edge behaviours moved, both in the safe direction:** - Sections that share a key: on the default arm, toggling a collapsible section could hide the fields of a non-collapsible twin whose row had no control. A non-collapsible section is now never collapsed. - The drawer no longer seeds its live state at mount, so a section whose declared `collapsed` changes after mount follows its declaration until the user toggles it, as ObjectForm always has. - `FormSectionContainer` (`FormSection.tsx`, a published React component) has the same pair shape at the prop level. The objectui#9780 report already recorded it as not class (c): it is a component prop, and its only caller (the wizard) passes neither. It is outside this file surface and not touched. - The pending `.changeset/9849-single-divider-projection.md` says of that change that 「every arm's gate, its collapse resolution and its predicate handling are unchanged」. That was a description of that change's diff, and it is still true of it. This changeset follows it in the same CHANGELOG and states the convergence. - `check:changeset-claims` asks for a read of `.changeset/8738-object-form-fields-description.md`, which names `ObjectForm.tsx`. It was read: it concerns the top-level `fields` vocabulary, which this diff does not touch, and it is still true. Dispatched by the `domain:ui` seat, session `https://claude.ai/code/session_01BA3nKVUwKQJf8DBxrSVtNC`, claim comment `5812052116`. --- _Generated by [Claude Code](https://claude.ai/code/session_01BA3nKVUwKQJf8DBxrSVtNC)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Fixes #10120
Clause-②: no — no exported symbol is added, removed, renamed or retyped, no key on a published payload moves, and no accept set is relaxed. Declared from THIS diff, not from the card:
sanitizeFormDatagains an optional third argument andfieldWriteGate/applyFieldPermissionsare new, but@object-ui/plugin-formpublishes.only, fromindex.tsx, which re-exports neither module. Measured on the builtdist/index.d.tswith a positive control in the same command:sanitizeFormData0 hits,fieldWriteGate/applyFieldPermissions0 hits,ObjectForm6 hits — so the zeros are measured absences, not a silent instrument.LookupField's props are unchanged.Clause-②:line (there was no diff yet) and named aClause-②-correction:as the protocol's second carrier. That correction is the seat's to post; this body is the declaration it corrects toward, and its value isno.What reproduced, and what did not
Everything on the card was relayed. Re-derived here at the seams, on
origin/mainatafb228418, with areporterprincipal whose permission set markskpi_entry_line.scorereadable: true, editable: false— the card's own fixture:actual_valueObjectForm(record page)scorealready ABSENTModalForm(record edit dialog)scorealready ABSENTDrawerFormscorePRESENT — reproducesLookupFieldchip, fielddisabled⇒ The card's premise is partly falsified and partly confirmed, and the difference is the finding. The two console entry points the card names — the record page and the record edit dialog — already carried a field-level strip and a render gate when this branch was cut, so the card's exact reproduction path no longer 403s there. What it was really reporting is a family that disagreed with itself: each container spelled its own COPY of both halves, and the third container of the family carried neither. That is the same defect class as objectui#10108 one layer down, and the drawer is reachable from the same authored
formTypekey the other two are.The lit control the card supplied held at every seam: the unchanged, un-denied columns (
indicator_name,target_value,weight) are still on the wire after the fix, so this is not "send less".The existing 「may this caller edit this field」 answer, reused
checkField(object, field, 'write')in@object-ui/permissions—MePermissionsProviderresolves it from the server's/me/permissionsenvelope by"OBJECT.FIELD"key, falling back to the object-levelallowEditfor a field no permission set mentions. ⛔ No rung of it is re-derived here.fieldWriteGateadapts that one verdict into the predicate the outbound filter takes, andapplyFieldPermissionsinto the field-list pass that decides what renders.What changed
sanitizeFormData's newcanEditoption) instead of a strip loop written out after each container's call — the loops were the thing that could be forgotten, and one of three had been.DrawerFormgains both halves;ObjectFormandModalFormproduce the payloads and the markup they produced before.isLoadedfalse): a standalone form, a designer preview and a guest surface have no resolvable principal, and the server still enforces.readonlyis folded intodisabledby the form's section builder, and a field the permission set refuses is marked disabled by the pass above. The chips stay — the value is readable — only the affordance goes.Ablation
Committed first, then mutated, then restored from
HEAD; both mutations verified to have landed on disk by counting the anchor text before and after, and the restore verified byte-identical (git hash-objectequals theHEADblob for both files, andgit diff HEADempty). Nodist/is involved: the root vitest config aliases every@object-ui/*specifier to that package'ssrc, so the mutation is what the tests load.canEditbranch fromsanitizeFormDataand the chip gate fromLookupField→ 7 failed / 9 passed, and the seven are exactly the payload rows (all three containers), the two filter rows and the two chip rows.Direction observed: turn-red, as expected. The two render-gate rows stayed green under this ablation because it does not touch
applyFieldPermissions; their before-state is the direct measurement in the table above, taken on unmodifiedorigin/main.Checks on this head
pnpm exec vitest run packages/plugin-form/ packages/fields/→ 280 files, 4001 passed, 1 skipped, exit 0 (through the shared verify lock).pnpm --filter @object-ui/plugin-form --filter @object-ui/fields type-check→ exit 0. Each package's script istsc --noEmit && tsc -p tsconfig.test.json, so the new test files are covered.pnpm --filter @object-ui/plugin-form --filter @object-ui/fields lint→ exit 0, 0 errors (906 pre-existing warnings in plugin-form, allno-explicit-any; the new files add only that same warning the packages already carry).check:control-bytes,check:changeset-presence,check-changeset-no-major,check:changeset-claims,check:pending-changeset-literals,check:new-line-citations(0 new citations),check:test-path-roots,check:vi-mock-override-shape,check:vi-mock-specifiers,check:vi-mock-inherit,check:unreferenced-sources,check:i18n-keys,check:i18n-dead-keys.check-governed-queue-guard --testover all nine paths: NOT GOVERNED.vite buildused for theClause-②.d.tsmeasurement was run outside it. Everything reported above is a real reading; nothing is inferred.Acceptance notes
Out of scope, seen but left alone by this branch and not filed from it:
LineItemsPanel— the child-row grid inside a master-detail form — consults no field-level permission at all when it decides which cells are editable. Not measured on this branch and not filed from it; the outbound half is partly covered already, because a master-detail child update sends only the cells that differ. Handed to the report for the triage seat to dedupe and file.ObjectFormRendererno-ops on afieldsentry that is a bare string rather than an object, which is the shape the console's global record form passes. Harmless today — every container re-applies the same pass to the resolved field list downstream, which is what covers that path — so it is an observation, not a defect. Noted, not filed. Successor: whoever unifies that outer gate with the per-container one.Generated by Claude Code