[finding] record:line_items issues getObjectSchema(undefined) for a panel whose childObject is unset, instead of declining to fetch

Found during the scope sweep for #5940 and deliberately left unfixed there — that card's scope was object-master-detail-form's detail-collection resolution, and #5940's dispatch order named other unresolved-sub-key sites as file-not-fix.

Fact (objectui `origin/main` @ `b37d3f0`)

LineItemsPanel (registered as record:line_items) reads schema.childObject and hands it to the data layer with no guard on it being resolved:

packages/plugin-form/src/LineItemsPanel.tsx:110-118

useEffect(() => {
  const ds: any = dataSource;
  if (!ds || typeof ds.getObjectSchema !== 'function') return;
  let cancelled = false;
  ds.getObjectSchema(schema.childObject)
    .then((s: any) => { if (!cancelled) setChildSchema(s ?? null); })
    .catch(() => { if (!cancelled) setChildSchema(null); });
  return () => { cancelled = true; };
}, [dataSource, schema.childObject]);

The effect guards the data source (ds, and that getObjectSchema is callable) but never the argument. A panel authored without childObject therefore asks the backend for an object literally named undefined; the .catch turns whatever comes back into a null schema, so the symptom is a silently unsanitized child grid rather than a visible error.

Why this is the same defect class as #5940, filed separately

Identical shape, identical key name, same package — and the same in-repo precedent settles it: RelatedList declines when it cannot scope ("has no referenceField/parentId — refusing to fetch all rows", RelatedList.tsx:498), and after #5940 MasterDetailForm's detail resolve effect declines on this exact key. MasterDetailForm's other effect — the child-schema cache — has always spelled it .filter(Boolean). So three sites in this package now decline and this one does not.

Not folded into #5940 because that would have widened a scoped card into a sweep, and because the two blocks differ in one way worth a reading before anyone edits: record:line_items is bound to an existing parent record, so whether an unset childObject is reachable at all through the registered authoring surface (vs. only through direct component use) has not been measured here. That measurement is what this card is for.

Not verified here

Whether the registry declares childObject as a required input for record:line_items, and whether the spec's own RecordLineItemsProps makes it required.
Whether a bare mount reproduces the call, the way [finding] object-master-detail-form issues getObjectSchema(undefined) for a detail collection whose object it never resolved, instead of declining to fetch #5940's premise was reproduced against the binding-reach fixture.

Related: #5940 (same decline-to-fetch decision, object-master-detail-form), #3838 and #3840 (same family — an unresolved optional sub-key reaching a read unguarded).

PM claim + dispatch order — R27

Claimed by the domain:devx @ objectui execution seat (#5748), PM session session_019b5UBNMtTzKbVtZZGvFuxe. Branch: claude/issue-6188-line-items-decline-to-fetch.

Ruling: decline to fetch, following the shape #5940 just landed. The decision is already made; what is open is a measurement.

Same defect, same key name, same package as #5940, and the same in-repo precedent settles it — RelatedList declines when it cannot scope (RelatedList.tsx:498), MasterDetailForm's child-schema cache has always spelled it .filter(Boolean), and after PR #6191 its detail-resolve effect declines on this exact key. Three sites in this package decline; this one does not. ⛔ There is no design question here to reopen.

⚠️ Read PR #6191's diff before you write anything and follow its shape rather than inventing one. It is in the merge queue as I write this, so it may or may not be on main when you claim — if it is not, read the PR. Two things in it are deliberate and you should match both:

It returns the entry rather than dropping it, because the surrounding array is index-matched against row state. ⭐ A .filter() looks cleaner, passes the obvious test, and silently corrupts state. Check whether the same index-coupling exists here before choosing.
The warning names the key and says what to set it to, rather than logging that something was undefined.

⭐ The measurement this card is actually for — do it first, and let it set the scope

The card is explicit that it did not verify reachability, and that is the interesting part:

record:line_items is bound to an existing parent record, so whether an unset childObject is reachable at all through the registered authoring surface (vs. only through direct component use) has not been measured here.

Measure it, and report the answer either way:

Does the registry declare childObject as a required input for record:line_items? Does the spec's RecordLineItemsProps make it required?
Does a bare mount actually reproduce the getObjectSchema(undefined) call, the way [finding] object-master-detail-form issues getObjectSchema(undefined) for a detail collection whose object it never resolved, instead of declining to fetch #5940's premise was reproduced against the binding-reach fixture?

⭐ If the call cannot be reached through the registered authoring surface, that does not kill the card — it changes what the fix is worth, and you must say so plainly. The guard is still correct (the component is public API and reachable directly), but the severity claim changes from "an author can trigger this" to "only direct component use can", and the report must not leave the stronger reading standing if the weaker one is what you measured. ⛔ Do not quietly ship the guard and let the card's framing imply a reachability you did not find.

If it is reachable through authoring, say how, and pin it the way #5940 did.

What the fix must prove

Predict each direction before running:

Before: reproduce the unguarded call with childObject unset. ⛔ If you cannot reproduce it by any route, stop and hand back — do not fix a call you could not observe.
After: the getObjectSchema(undefined) call is absent, and a well-formed panel still fetches. ⭐ Both halves — a guard that declines everything also makes the bad call vanish and would pass an absence-only assertion.
A regression assertion on the call list, not on call presence.

⚠️ Note the symptom differs from #5940's and that shapes the test: here the .catch turns the result into a null schema, so the visible outcome is a silently unsanitized child grid rather than an error. An assertion that only checks "no crash" or "schema is null" would have passed while the defect was live — .catch already produced null. Assert on the call, not the resulting state.

Scope

packages/plugin-form/src/LineItemsPanel.tsx and its test. ⛔ Not MasterDetailForm.tsx (#5940/#6191 owns it — and touching it would collide with a PR in the queue). ⛔ Not #3838, ⛔ not #3840. Any further unguarded sub-key site you find: file it, do not fix it — that is how this card came to exist, and the class is now at three instances.

Root vitest only per objectui#3378; exit codes captured by redirect before any pipe; derive each gate's scope from its own configuration rather than assuming.

Generated by Claude Code

os-dev-report

{
  "issue": 6188,
  "status": "done",
  "branch": "claude/issue-6188-line-items-decline-to-fetch",
  "pr": "https://github.com/objectstack-ai/objectui/pull/6196",
  "premise_still_valid": true,
  "summary": "THE MEASUREMENT, first, and it lands on the STRONGER reading: `childObject` IS declared `required: true` on the registry entry for `record:line_items` (index.tsx:404) but NOTHING enforces it — `inputs[].required` is designer metadata (WidgetRegistry.ts:180 copies it onto the ComponentRegistry entry; every consumer of `.required` under packages/core/src is field-level or record-level, none sees a block's declared inputs), and there is NO `RecordLineItemsProps` anywhere: grep over @objectstack/spec@17.2.0 src+dist and over this whole repo returns zero hits, and the spec names this block exactly once as an example of the opposite ('`type` is an open union (unregistered types like `record:line_items` are authored in the wild)', component.zod.ts:105). So the block has no spec schema at all. A bare authored node mounted through SchemaRenderer — the real registered path, through ComponentRegistry and the ElementDataSourceGate wrapper — reproduced the call. The severity claim therefore stands as the card framed it: an author who omits the key triggers this, not only direct component use. FIX: the child-schema effect declines and warns, naming the key and what to set it to, and clears the cached child schema (an unresolvable panel HAS no child schema, and a stale one would sanitize a later save against the wrong object's fields; `null` is what the effect's own `.catch` already produced, so the sanitize path needs no new case). #6191's index-coupling choice was CHECKED, not copied: there is no array here — one panel, one `childSchema` state — so 'return the entry rather than drop it' has no analogue, and the state question above is what replaces it.",
  "tests": "All at fd7525d9b, the commit the PR ships. Exit codes captured by redirect before any pipe; heavy runs through scripts/pm/os-verify-lock.sh (VERDICT lines quoted, not bare $?).\n\nBEFORE leg (new test vs UNMODIFIED source), predicted before running and matched exactly: EXIT=1, 'Tests 1 failed | 1 passed (2)', AssertionError 'expected [ getObjectSchema(undefined), ...(1) ] to deeply equal [ find(undefined) ]' with received ['getObjectSchema(undefined)', 'find(undefined)']. The console.warn assertion also had nothing to satisfy it. Test 2 (well-formed panel) passed unchanged.\n\nAFTER leg (same file vs guarded source): EXIT=0, 'Test Files 1 passed (1) / Tests 2 passed (2)'. Both halves pinned — the bad call is ABSENT and a well-formed panel STILL fetches — because a guard that declined everything would also pass an absence-only assertion.\n\nRegression assertion is on the CALL LIST, not call presence: `expect(calls).toEqual([...])` over `method(firstArgument)` recordings. Deliberately the first argument only, not the full argument list: the child fetch also carries $filter/$top and pinning those would fail for changes unrelated to the object name. Assertions on resulting state were impossible here — the `.catch` already produced a null child schema, which is also what the panel holds before any fetch resolves, so 'no crash' and 'schema is null' were both TRUE while the defect was live.\n\nUNION at fd7525d9b: (1) vitest, root config, path-filtered per objectui#3378 — packages/plugin-form/ plus apps/console record-block-record-reach + public-block-binding-reach + public-contract: 'Test Files 64 passed (64)' / 'Tests 655 passed (655)', lock VERDICT command-exit 0. (2) turbo run type-check --filter=@object-ui/plugin-form --concurrency=2: 'Tasks: 13 successful, 13 total' (dependency build included, plus tsconfig.test.json which type-checks the new test), lock VERDICT command-exit 0. (3) eslint . in packages/plugin-form: exit 0, '641 problems (0 errors, 641 warnings)'. (4) node scripts/check-control-bytes.mjs: exit 0, 'OK (scanned 5112 tracked text file(s); skipped 85 binary)'; plus a direct control-byte grep over the three changed files, no hits. (5) pnpm run changeset:check: exit 0, both checks OK.\n\nGATE SCOPE DERIVED FROM EACH WORKFLOW'S OWN CONFIG, not assumed: ci.yml and lint.yml carry paths-ignore for **/*.md, content, docs, .changeset only, so a packages/** diff runs both in full; control-bytes.yml and changeset-presence.yml deliberately carry no path filter; changeset-guard.yml filters to .changeset/**, which this PR touches. performance-budget.yml (Bundle Analysis) filters on packages/** and so APPLIES — it is a full turbo build plus a console build, a repo-level run CI owns, NOT reproduced locally. LINT NARROWING IS A MEASUREMENT: the universe comes from eslint's own resolution (`eslint .` from the package dir, which is what turbo run lint invokes — packages/plugin-form/package.json 'lint': 'eslint .'), and the root eslint.config.js declares no `project`/`projectService`, so type-aware linting is OFF and this diff cannot move the verdict on any file it does not touch.\n\nLINT NOTE, stated because it is a new warning I introduced: setChildSchema(null) raises one react-hooks/set-state-in-effect WARNING — the same one the effect below it already carries (void load(), line 203); the rule is 'warn' repo-wide (eslint.config.js:95) and lint.yml deliberately sets no --max-warnings (its own comment, line 20). 0 errors.\n\nCI not awaited, per dispatch contract: report is delivered at draft-PR time.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #6194: `record:line_items` also issues `find(undefined, …)` for a panel whose `childObject` is unset — the SIBLING site in the same component (`load()` guards dataSource and parentId but not the object being queried). Measured in the same run: after this PR the recorded list is ['find(undefined)']. Filed not fixed per dispatch order; it is PINNED as current behaviour in the new test with a comment naming #6194 and what to change the line to, rather than hidden behind a narrower fixture that omits parentId — hiding it would have made this PR's test read as if the component were clean. NOT a one-line rider: declining the ROW fetch has a visible consequence the schema fetch does not — `load` owns `loading`, and the render branches loading -> !parentId -> grid, so an unresolvable panel with a parent id bound lands on the grid branch and shows an empty editable grid over a nonexistent object; whether that wants its own config-hint branch is a question, not a mechanical fix. The class is now at four instances (#3838, #3840, #5940/#6191, #6188) plus #6194.",
    "NOT filed, recorded here only: the save path hands `childObject: schema.childObject` to buildMasterDetailEditBatch unconditionally (LineItemsPanel.tsx:205). Plausibly the same shape, but the Save button is disabled until the grid is dirty, so reaching it requires editing rows of the nonexistent object first. NOT measured — noted inside #6194's 'Not verified here' rather than filed separately, so it is triaged with the site it belongs to instead of becoming a second card for one component."
  ]
}

Generated by Claude Code

PM review — ACCEPTED. PR #6196 armed (ready + auto-merge SQUASH). 17 green, 0 failures.

Reviewed against a pinned ref.

⭐ The measurement answered the card's open question and landed on the stronger reading

I said explicitly that a negative result would not kill the card but would change the severity claim, and that the report must not leave the stronger reading standing if the weaker one was what got measured. The measurement went the other way, and it is more interesting than either option I framed:

childObject IS declared required: true on the registry entry (index.tsx:404) — and nothing enforces it. inputs[].required is designer metadata: WidgetRegistry.ts:180 copies it onto the entry, and every consumer of .required under packages/core/src is field- or record-level. None sees a block's declared inputs.
⭐ There is no RecordLineItemsProps at all — zero hits across @objectstack/spec@17.2.0 src and dist and across this whole repo. The card asked whether the spec makes it required; the answer is that the block has no spec schema whatsoever, and the spec names it exactly once as an example of the opposite: "unregistered types like record:line_items are authored in the wild".
A bare authored node through SchemaRenderer — the real registered path — reproduced the call.

So a required-marked key, with no enforcement and no schema, reached the data layer as undefined. The severity claim stands as the card framed it: an author triggers this, not only direct component use.

⭐ The precedent was checked, not copied — which is what I asked for and rarely get

I told them to follow PR #6191's shape, and flagged its index-coupling choice (return the entry, don't drop it). The right response was not to imitate it:

there is no array here — one panel, one childSchema state — so "return the entry rather than drop it" has no analogue, and the state question above is what replaces it.

And they found the real analogue: clear the cached child schema. An unresolvable panel has no child schema, and leaving a previous object's schema in place would sanitize the next save against the wrong object's fields. null is already what the .catch produces, so the sanitize path needs no new case.

That is a correctness argument reasoned from this component, not a pattern applied because a sibling used it. Copying #6191's return verbatim would have left a stale schema behind and been defensible-looking.

⭐ The trap I warned about was confirmed empirically, not just avoided

I said the .catch already produces a null schema, so an assertion on resulting state would have passed while the defect was live. Measured:

"no crash" and "schema is null" were both TRUE while the defect was live

— because null is also what the panel holds before any fetch resolves. So there was no state assertion available at all, and the call list was the only place the defect was observable. The before leg reproduced it exactly: ['getObjectSchema(undefined)', 'find(undefined)'].

The assertion pins the first argument only, deliberately — the child fetch also carries $filter/$top, and pinning those would red on changes unrelated to the object name. A regression pin that reds for the wrong reasons gets deleted.

⭐ The out-of-scope finding, and the judgement call inside it

#6194 — the sibling site in the same component issues find(undefined, …): load() guards the data source and parentId but not the object being queried. Filed, not fixed, per the order.

But the part I want to single out: they pinned it as current behaviour in the new test, with a comment naming #6194 and what to change the line to, rather than choosing a narrower fixture that omits parentId and hides it. Their reason:

hiding it would have made this PR's test read as if the component were clean.

⭐ That is the correct instinct and it is the opposite of what most scoping pressure pushes toward. A test that quietly avoids the known-broken path is how a component acquires a reputation for being covered.

They also explain why #6194 is not a one-line rider: load owns loading, and the render branches loading → !parentId → grid, so an unresolvable panel with a parent id bound lands on the grid branch and shows an empty editable grid over a nonexistent object. Whether that wants its own config-hint branch is a question, not a mechanical fix. Correct to stop there.

The class is now at five instances: #3838, #3840, #5940/#6191, #6188, #6194.

Verification and declarations

Both halves pinned — bad call absent, well-formed panel still fetches. Gate scope derived from each workflow's own config, including the observation that performance-budget.yml filters on packages/** and therefore applies (a repo-level build CI owns, correctly not reproduced locally). Lint narrowing measured from eslint's own resolution, with no type-aware linting configured.

⭐ And the new warning they introduced was declared rather than buried: setChildSchema(null) raises one react-hooks/set-state-in-effect warning — the same one the adjacent void load() already carries, rule is warn repo-wide, and lint.yml deliberately sets no --max-warnings. Saying "I added a warning and here is why it is acceptable" beats letting a reviewer discover it.

Noted: the PR base advanced to ead74abe during creation (mergeable_state: behind), with no file overlap with #6191, which touched MasterDetailForm.tsx only. The queue rebuilds on current main, so that resolves itself.

Generated by Claude Code

Navigation Menu

Repository navigation

[finding] `record:line_items` issues `getObjectSchema(undefined)` for a panel whose `childObject` is unset, instead of declining to fetch #6188

Fact (objectui `origin/main` @ `b37d3f0`)

Why this is the same defect class as #5940, filed separately

Not verified here

PM claim + dispatch order — R27

Ruling: decline to fetch, following the shape #5940 just landed. The decision is already made; what is open is a measurement.

⭐ The measurement this card is actually for — do it first, and let it set the scope

What the fix must prove

Scope

PM review — ACCEPTED. PR #6196 armed (ready + auto-merge SQUASH). 17 green, 0 failures.

⭐ The measurement answered the card's open question and landed on the stronger reading

⭐ The precedent was checked, not copied — which is what I asked for and rarely get

⭐ The trap I warned about was confirmed empirically, not just avoided

⭐ The out-of-scope finding, and the judgement call inside it

Verification and declarations

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[finding] record:line_items issues getObjectSchema(undefined) for a panel whose childObject is unset, instead of declining to fetch #6188

Description

Fact (objectui origin/main @ b37d3f0)

Why this is the same defect class as #5940, filed separately

Not verified here

Activity

yinlianghui-tw commented on Aug 24, 2026

PM claim + dispatch order — R27

Ruling: decline to fetch, following the shape #5940 just landed. The decision is already made; what is open is a measurement.

⭐ The measurement this card is actually for — do it first, and let it set the scope

What the fix must prove

Scope

yinlianghui-tw commented on Aug 25, 2026

yinlianghui-tw commented on Aug 25, 2026

PM review — ACCEPTED. PR #6196 armed (ready + auto-merge SQUASH). 17 green, 0 failures.

⭐ The measurement answered the card's open question and landed on the stronger reading

⭐ The precedent was checked, not copied — which is what I asked for and rarely get

⭐ The trap I warned about was confirmed empirically, not just avoided

⭐ The out-of-scope finding, and the judgement call inside it

Verification and declarations

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[finding] `record:line_items` issues `getObjectSchema(undefined)` for a panel whose `childObject` is unset, instead of declining to fetch #6188

Fact (objectui `origin/main` @ `b37d3f0`)