Skip to content

finding(plugin-form): the same top-level fields silent-drop trap is undocumented on TWO more registrations — form/view:form and object-master-detail-form #8847

Closed

Description

@os-warren

Context

objectui#8738 (route 2, closed by objectui#8846) documents object-form's
top-level fields input: members are bare field names, { name } tolerated,
and the spec FormFieldSchema object ({ field: 'note', colSpan: 2 }) —
legal in sections[].fields — resolves to no name and is silently skipped
(SimpleObjectForm in ObjectForm.tsx; buildFlatFields in flatFields.ts).

Re-deriving the read sites for #8738 (against main @ e9d92120a) turned up
the identical, still-undocumented trap on two more ComponentRegistry
registrations in packages/plugin-form/src/index.tsx:

  1. packages/plugin-form/src/index.tsx:292 —
    ComponentRegistry.register('form', ObjectFormRenderer, …), the
    view:form alias of the exact same ObjectFormRenderer / SimpleObjectForm
    component object-form uses. Its { name: 'fields', type: 'array' } entry
    has no description at all — the same gap finding(plugin-form): object-form carries TWO surfaces spelled fields with different member vocabularies — the spec FormFieldSchema object is silently dropped by the top-level one #8738 closed for object-form,
    left open here.
  2. packages/plugin-form/src/index.tsx:~430 —
    object-master-detail-form's fields input, documented as 'Which parent fields to show, in order. Ignored when sections is given — sections carry their own field lists.'. MasterDetailForm.tsx's own comment says this is
    'Parent form sections/fields — passed straight through to ObjectForm' —
    i.e. the same SimpleObjectForm read site — so the same
    { field: 'note' } silent-drop applies, and the existing description does
    not mention it.

Why this is a category-(c) trap, not a nit

Both entries let an author (or an AI generating metadata) write the spec's
FormFieldSchema object into a fields array that looks identical in shape
to sections[].fields, and get a silent drop with no throw, warning, or
empty-state — exactly the population #8738 was opened to close, just reached
through a different registered type name.

Suggested fix

Same route-2 treatment as #8846: add/extend the description on both entries
to state the bare-name-only vocabulary and the contrast with
sections[].fields, mirroring the text #8846 added to object-form.fields.
Route 1 (whether to also console.warn) is out of scope here too — that
question is still unruled for #8738 and this finding doesn't change that.

Not filed as part of #8846

#8738's dispatch and its pinned probe
(objectFormFieldsMembers-8071.test.tsx) scope specifically to object-form;
widening that PR's diff to other registrations wasn't authorized, so this is
filed separately rather than folded in.

Activity

  1. added
    bugSomething isn't working
    domain:specobjectui spec stream: fix lands on packages/types, schema corpus or spec pin coupling — spec lane
    on Sep 9, 2026
  2. self-assigned this
    on Sep 9, 2026
  3. os-warren commented on Sep 9, 2026

    @os-warren
    CollaboratorAuthor

    Claim: session session_01Jmxdo7bmeqCQHLSfmLVX9w (PM seat domain:spec, dispatching os-dev) · branch claude/issue-8738-fields-warn-route1
    Clause-②: no

    ⚠️ Dispatched together with objectui#8738 route 1 on one branch — the full claim is on #8738. Same package, same trap; ⛔ two devs in packages/plugin-form/src/index.tsx at once is a collision.

    ⭐ These two cards may be one fix. Route 1 adds a named console.warn in the shared read path, and this card's two surfaces (form / view:form at index.tsx:292, and object-master-detail-form's parent fields at :346) go through it. If the warning covers them, this card's remaining work shrinks to its description half. The dev measures that rather than assuming it either way.

    Confirmed by this seat on origin/main after PR #8846 merged — both are still bare, with no description, against the now-described object-form entry as the control:

    :232  { name: 'fields', type: 'array', description: 'Bare field names to show, in order …' }   ← described (#8846)
    :292  { name: 'fields', type: 'array' },                                                        ← bare
    :346  { name: 'fields', type: 'array' },                                                        ← bare
    

    Refs objectstack-ai/objectui#8847 unless the dev's measurement shows both halves are fully closed — then Fixes, decided on its own head, ⛔ not predicted here.


    Generated by Claude Code

  4. os-warren commented on Sep 9, 2026

    @os-warren
    CollaboratorAuthor

    ⛔ CORRECTION — this card names the wrong second surface, and I repeated the error in my own claim

    domain:spec @ objectui PM seat, session session_01Jmxdo7bmeqCQHLSfmLVX9w. ⛔ The card body above is not edited; the correction sits beside it.

    The card says the trap sits on form / view:form (index.tsx:292) and on object-master-detail-form's parent fields (~:430). My claim comment 5604074045 repeated that, and printed :346 with the label object-master-detail-form.

    Measured on origin/main 8a388ee76 — each fields registration input attributed to its nearest preceding ComponentRegistry.register( and that call's actual type key, ⛔ not to a nearby namespace: line (my first attempt used the namespace: heuristic and got a different, wrong answer — it is not the instrument):

    registrations:  :226 object-form   :285 form   :337 embeddable-form
                    :358 form-analytics   :402 object-master-detail-form   :499 line_items
    
    :232  described (PR #8846)  -> object-form
    :292  ⛔ BARE                -> form
    :346  ⛔ BARE                -> embeddable-form     ← NOT object-master-detail-form
    :430  described             -> object-master-detail-form
    

    ⇒ object-master-detail-form was never bare. It has carried "Which parent fields to show, in order. Ignored when sections is given…" all along. The genuinely undescribed second surface is embeddable-form.

    ⭐ The round's dev got this right in its residue note while the card and I had it wrong — it flagged embeddable-form at :346 specifically. Recording that because a card that misnames its own subject sends the next seat to the wrong file.

    Current state after PR #8859 (draft, in CI)

    Hunk headers name the targets unambiguously:

    @@ -289,7 +289,7 @@ ComponentRegistry.register('form', ObjectFormRenderer, {          ← described
    @@ -427,7 +427,7 @@ ComponentRegistry.register('object-master-detail-form', …      ← existing description EXTENDED with the vocabulary note
    

    Bare { name: 'fields', type: 'array' }, count: main 2 → branch head 1. Control: 4 fields inputs total on the branch head, so the instrument is not matching everything.

    surface warning (route 1) description
    object-form ✅ inherits ✅ PR #8846
    form ✅ inherits (literally the same ObjectFormRenderer) ✅ PR #8859
    object-master-detail-form ✅ inherits (parent fields with no sections falls into the same SimpleObjectForm branch — pinned by the new topLevelFieldsWarnCoverage-8847.test.tsx rendering MasterDetailForm directly) ✅ extended by #8859
    embeddable-form ✅ inherits (EmbeddableForm passes config.fields straight into <ObjectForm> with no sections) ⛔ still bare — this card's remainder

    ⇒ Refs, not Fixes, is correct and the dev's call to leave it open was right. The remainder is one description string on embeddable-form.

    ⭐ Note what the coverage measurement bought: route 1's warning was measured to reach all four surfaces rather than assumed, and the master-detail path got a render-level pin instead of an argument. ⇒ this card's remaining work is documentation-only, on one line.

    ⚠️ Dedup channel was down for the round. REST /search/issues is 403 by policy for these sessions — verified by this seat just now against two known-true controls: "This GitHub API path is not available: sessions are bound to their configured repositories." The dev also reported MCP search_issues returning total_count: 0 against known-true controls. ⇒ it correctly handed the residue up rather than filing blind. ⛔ No duplicate card filed; the residue is recorded here, on the card that already owns this trap, which needs no dedup at all.


    Generated by Claude Code

  5. removed their assignment
    on Sep 9, 2026
  6. self-assigned this
    on Sep 9, 2026
  7. os-warren commented on Sep 9, 2026

    @os-warren
    CollaboratorAuthor

    Claim: session session_01Jmxdo7bmeqCQHLSfmLVX9w (PM seat domain:spec, dispatching os-dev) · branch claude/issue-8847-embeddable-form-fields-description
    Clause-②: no

    The remainder, and it is small on purpose. PR #8859 closed this card's warning half for all four surfaces — measured, and pinned for the master-detail path by a test rendering MasterDetailForm directly. What is left is one description string.

    Measured on origin/main, each entry attributed to its real registered type key

    ⛔ Not by a nearby namespace: line — that heuristic gave me a wrong answer once already on this card (5604654332):

    :226 object-form   :285 form   :337 embeddable-form   :358 form-analytics
    :402 object-master-detail-form   :499 line_items
    
    :232  described (#8846)  -> object-form
    :292  described (#8859)  -> form
    :346  ⛔ BARE            -> embeddable-form            ← the remainder
    :430  described (#8859)  -> object-master-detail-form
    

    Bare { name: 'fields', type: 'array' }, count: 1. Control: 4 fields inputs total, so the matcher is not matching everything.

    ⚠️ This card's body names the wrong second surface — it says object-master-detail-form, which was never bare. Corrected at 5604654332. ⛔ Read the correction, not the body.

    Scope — one line

    Give embeddable-form's fields registration input a description, modelled on the two that already have one. It must state that members are bare field names ({ name } tolerated), and that this is not the sections[].fields vocabulary, which also accepts the spec FormFieldSchema object (identity key field) — a shape that resolves to no name here and is silently skipped.

    ⚠️ EmbeddableForm passes config.fields straight into <ObjectForm> with no sections, so it already inherits the runtime warning from PR #8859. Verify that rather than assuming it — if the warning does not reach this surface, that is a finding worth more than the description, and you should report it before writing prose that claims otherwise.

    Fixes objectstack-ai/objectui#8847 — confirm on your own head that no bare fields input remains. Changeset required (@object-ui/plugin-form is published; inputs[].description is an authoring surface).

    ⛔ Do not add a ?? f.field fallback, a second warning, or any behaviour — the card's standing prohibition. This round is description text only.


    Generated by Claude Code

  8. os-warren commented on Sep 9, 2026

    @os-warren
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 8847,
      "status": "done",
      "branch": "claude/issue-8847-embeddable-form-fields-description",
      "pr": "https://github.com/objectstack-ai/objectui/pull/8869",
      "premise_still_valid": true,
      "summary": "Added the missing `description` to embeddable-form's top-level `fields` registration input in packages/plugin-form/src/index.tsx, modelled on object-form (:232) and form (:292). States members are bare field names ({ name } tolerated) and contrasts with sections[].fields' spec FormFieldSchema object (identity key `field`), which resolves to no name here and is silently skipped. Description text only, no behaviour/accept-set/key/shape change. Also measured (throwaway render probe, not committed) that PR #8859's console.warn already reaches embeddable-form via inheritance through ObjectForm/SimpleObjectForm.",
      "tests": "Re-derived attribution independently on origin/main@42ddd7f71 by grepping ComponentRegistry.register(...) call sites and attributing each `fields` input to the nearest preceding call: :226 object-form / :285 form / :337 embeddable-form / :358 form-analytics / :402 object-master-detail-form / :499 line_items; fields inputs at :232 (described->object-form), :292 (described->form), :346 (BARE->embeddable-form), :430 (described->object-master-detail-form). Bare count=1, control=4 total fields inputs — matches the corrected card exactly. After edit: 0 bare, control still 4 (grep -c on the broader pattern). Warning-reach measurement: wrote a throwaway vitest render test (packages/plugin-form/src/__tests__/tmp-embeddable-form-warn-probe.test.tsx, deleted before commit, never in the diff) rendering EmbeddableForm (config.fields=[{field:'note'}], with a dataSource stub) — console.warn fired exactly once with the same message text as the object-form pin ('top-level `fields` entry { field: \\'note\\' } resolved to no field name...NOT the same vocabulary as `sections[].fields`...'); firing control with fields:['note'] fired 0 warnings. Confirms embeddable-form DOES inherit the warning (EmbeddableForm.tsx builds schema={type:'object-form',fields:config.fields,...} with no sections/formType and renders the ObjectForm component directly, which falls through to SimpleObjectForm regardless of registered type key). Pinned probe (must-not-move): `pnpm exec vitest run packages/plugin-form/src/__tests__/objectFormFieldsMembers-8071.test.tsx --reporter=verbose` -> exit 0, Test Files 1 passed (1), Tests 8 passed (8), same 8 test names/order as before my edit. Package regression: `pnpm exec vitest run packages/plugin-form/` -> exit 0, 89 files / 883 passed | 1 skipped (884). Sweep (apps/console, required): `pnpm exec vitest run apps/console/` -> exit 0, 98 files / 1132 passed. Typecheck: `pnpm --filter '@object-ui/plugin-form' run type-check` (tsc --noEmit && tsc -p tsconfig.test.json) -> exit 0, silent. Build: dependency closure `turbo run build --filter='@object-ui/plugin-form^...' --concurrency=2` -> exit 0, 11/11 successful; then `pnpm --filter '@object-ui/plugin-form' build` -> exit 0; grepped packages/plugin-form/dist/index.js and confirmed the new description string reached the built output (4 occurrences of 'silently skipped', matching all 4 now-described fields inputs). Ledger check (objectui#8614 hazard): `node scripts/check-doc-example-types.mjs` -> exit 2, but solely on missing dist/*.d.ts for ~26 unrelated packages (a full-workspace build precondition, out of this diff's local-verification scope per this repo's own build-then-judge convention); directly grepped the UNGATED_EXAMPLES ledger body (scripts/check-doc-example-types.mjs) for 'plugin-form' — zero matches, so today's re-keying by sibling PRs cannot affect this file regardless. My edit is also line-count-neutral (git diff --stat: 1 insertion, 1 deletion; wc -l unchanged at 511), so no line-shift risk either way. Changeset gates: `node scripts/check-changeset-presence.mjs` -> exit 1 before adding a changeset (correctly demanded one for @object-ui/plugin-form's packages/plugin-form/src/index.tsx), exit 0 after adding .changeset/8847-embeddable-form-fields-description.md (patch bump). `node scripts/check-changeset-no-major.mjs` -> exit 0. Lint (scoped to touched file only, full-workspace lint left to CI per local-verification-scope discipline): `npx eslint packages/plugin-form/src/index.tsx --no-inline-config` -> exit 0, 25 pre-existing warnings (react-refresh/only-export-components, @typescript-eslint/no-explicit-any), none on the touched line 346, 0 errors. Control-byte self-scan on the touched file (grep -naP control-char class) -> clean, no matches. Governed-surface check: `node scripts/check-governed-queue-guard.mjs --test packages/plugin-form/src/index.tsx .changeset/8847-embeddable-form-fields-description.md` -> NOT GOVERNED. Base->origin/main pre-flight: empty window (BASE=42ddd7f71 == origin/main both at worktree creation and again immediately before push).",
      "mcp_calls": 4,
      "open_questions": [],
      "out_of_scope_findings": ["noted, not filed: object-form/form/object-master-detail-form's existing descriptions (added by #8846/#8859) all say 'silently skipped' without mentioning that route 1's console.warn now fires at that same read site — consistent precedent I followed for embeddable-form's new description too, but if the maintainer wants descriptions to also name the runtime warning, all four entries (including this PR's) would need a follow-up in one pass rather than drifting one at a time. Carrier: none identified — would need its own card."]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:specobjectui spec stream: fix lands on packages/types, schema corpus or spec pin coupling — spec lanepm:dispatched

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions