Repository navigation
[finding] object-master-detail-form issues getObjectSchema(undefined) for a detail collection whose object it never resolved, instead of declining to fetch #5940
Description
Activity
- addeddomain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repoobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repo
on Aug 24, 2026 yinlianghui-tw commented
on Aug 24, 2026 CollaboratorMore actionsPM claim + dispatch order — R18
Claimed by the
domain:devx@ objectui execution seat (#5748), PM sessionsession_019b5UBNMtTzKbVtZZGvFuxe. Branch:claude/issue-5940-detail-collection-decline-to-fetch.Ruling: the product half is the card. Take it. The fixture half is taken only if the measurement says the sample is malformed.
The card declines to pick between its two halves and asks for the discriminating step. Here it is, and the product half does not actually depend on the answer:
A renderer that cannot resolve a detail collection's object must decline to fetch, not call
getObjectSchema(undefined).That is not a judgement call in this repo —
record:related_listalready sets the precedent, declining when it has no parent id, and the card names it. An in-repo precedent for the identical decision is the strongest form of ruling available, and it means "should the renderer guard?" is already answered onmain; only this component missed it. A real backend receives a query for an object literally namedundefined, and whatever it returns becomes the console's problem.So: the product fix lands regardless of what the fixture turns out to be. That is why this is dispatchable rather than a decision card.
The discriminating step the card did not spend — do it first, it decides the second half
Measure, do not infer:
- Which key does the renderer read the child object off? The card says
.object/.objectNamewithout settling it. Read the call site. - What is a
detailsentry declared to be?object-master-detail-form's spec input is{ name: 'details', type: 'array', ... }. Read the declared entry shape — from the type/spec, not from the sample.
Then:
- If a detail entry is declared as an object with a child-object key →
['name']is a malformed sample. Fix it in the probe's sample table too, the same way [finding]object-form与object-master-detail-form在 binding-reach 探针的自动 fixture 下渲染成错误卡(reading 'map')—— 断言仍成立,但两个分支的崩溃守卫因此没法一起开 #3840 landed thesectionssample: on the declared shape, ⛔ not on "the bad call went away". - If a detail entry really is a bare string → the sample is fine and the fixture half is closed by measurement rather than by edit. Say so explicitly in the report; a half closed by a reading is a result, not a skip.
⚠️ Two things that will bite- Branch from today's
origin/main. PR fix(plugin-form): declare object-master-detail-form formType as a closed, measured vocabulary #6177 (card [finding]object-master-detail-formdeclaresformTypeas a barestringwhile the renderer switches on theobject-formvariant enum — an out-of-vocabulary value silently routes around the whole simple-form path #5939) landed tonight onpackages/plugin-form/src/index.tsx— the same component's spec — tighteningformTypefrom a barestringto a closedenum. You will be reading the file it just changed. Re-derive; do not trust any quotation of that file older than 22:58Z. - This call is invisible to the binding-reach probe's assertions. That probe asks whether any call carried the object name, and the first, correct call already satisfies it. So ⛔ a green probe is not evidence your fix worked — it was green while the defect was live. You need an assertion that reads the full call list, which is how the defect surfaced in the first place.
What the fix must prove
Predict each direction before running:
- Before: mounting the fixture records exactly two calls, the second
getObjectSchema(undefined). Reproduce this first — if you cannot, the premise is stale and you stop and hand back. - After: the second call is absent, and
getObjectSchema("probe_object__c")is still made. ⭐ Both halves matter: a "fix" that declines to fetch everything would also make the bad call disappear, and would pass a test that only checks for absence. - A regression assertion on the call list, not on call presence — otherwise nothing stops this reopening exactly as it opened.
Follow whatever shape
record:related_list's decline uses, so the two components fail the same way rather than two ways.Scope
The renderer's detail-collection resolution, its test, and — conditionally, per the measurement above — the probe's sample table. ⛔ Not #3838, ⛔ not the binding-reach probe's own assertions, ⛔ not any other unresolved-sub-key site the sweep turns up: file those.
Generated by Claude Code
- Which key does the renderer read the child object off? The card says
yinlianghui-tw commented
on Aug 24, 2026 CollaboratorMore actions{ "issue": 5940, "status": "done", "branch": "claude/issue-5940-detail-collection-decline-to-fetch", "pr": "https://github.com/objectstack-ai/objectui/pull/6191", "premise_still_valid": true, "summary": "BOTH halves taken; the fixture half was decided by measurement, not assumed. Discriminating step: the renderer reads the child object off `childObject` (neither `.object` nor `.objectName` as the card guessed), and a `details` entry is declared as MasterDetailDetailConfig — an OBJECT whose `childObject: string` is REQUIRED, not a bare string. So `['name']` IS a malformed sample and the probe's sample table is fixed too, decided on the DECLARED SHAPE rather than on the bad call going away. Product half: the detail resolve effect now skips an entry with no `childObject` and warns, leaving it in place exactly as the adjacent `catch` does — following RelatedList's decline (\"has no referenceField/parentId — refusing to fetch all rows\"), which the sibling child-schema-cache effect in this very component already spelled `.filter(Boolean)`. Three sites in the package now fail the same way instead of three ways.", "tests": "Verified at commit fd8df8399, root vitest only (objectui#3378). Exit codes captured by redirect before any pipe; each gate quoted from its own verdict line. BEFORE (predicted two calls, second getObjectSchema(undefined)) — reproduced exactly: [\"getObjectSchema(\\\"probe_object__c\\\")\", \"getObjectSchema(undefined)\"]. AFTER (predicted second absent AND first still made) — both hold: [\"getObjectSchema(\\\"probe_object__c\\\")\"]. REVERSE VERIFICATION: fix committed FIRST, then reverted alone via `git checkout HEAD~1 -- MasterDetailForm.tsx` inside a script with `trap '<restore>' EXIT INT TERM`; mutation confirmed ON DISK by grepping the text intended to be removed (`if (!d.childObject)` -> 0, warn string -> 0), NOT by the edit tool's exit code; predicted red on test 1 only and observed exactly that — `AssertionError: expected [ ...(2) ] to deeply equal [ Array(1) ]`, `Tests 1 failed | 1 passed (2)`; test 2 correctly stayed green (a well-formed detail was always fetched, so it does not depend on the guard); restore leg confirmed byte-identical (`git status` empty vs the commit). No dist/build ablation applies — this is a source-resolved unit suite, and the reverted leg was re-run against the same resolution path. GATES: `pnpm lint` full repo (eslint . --no-inline-config) ran IN FULL under the shared verify lock — `VERDICT command-exit 0 · held the lock 178s`, `Tasks: 47 successful, 47 total`, `0 errors` — so NO narrowing is claimed. `pnpm --filter @object-ui/plugin-form type-check` exit 0 (script name echoed, so not a zero-match silent pass), after building the dependency closure — the first attempt failed with TS2307 module-resolution errors purely from an unbuilt closure. `pnpm --filter @object-ui/console type-check` — `VERDICT command-exit 0`. packages/plugin-form — `Test Files 61 passed (61)`, `Tests 615 passed (615)`. public-block-binding-reach after the sample change — `Tests 16 passed (16)`. Sibling parity suites (record-block-record-reach, registry-inputs-spec-parity, masterDetailFormTypeManifest, public-contract, catalog-gallery-render) — `Tests 672 passed (672)`. UNION re-run on the FINAL commit fd8df8399 — `Test Files 63 passed (63)`, `Tests 644 passed (644)`. `pnpm check:control-bytes` — `OK (scanned 5110 tracked text file(s); skipped 85 binary)`, plus a self-scan of all four changed files with grep -naP over the control-byte class: clean. `pnpm changeset:check` — all three checks OK. Branched from today's origin/main b37d3f0, which already carries PR #6177; the formType enum was read from the file as it now stands, not quoted from before 22:58Z.", "open_questions": [], "out_of_scope_findings": [ "filed as #6188: `record:line_items` (LineItemsPanel.tsx:114) reads `schema.childObject` through the identical unguarded path — the effect guards the data source and that getObjectSchema is callable, but never the argument; same defect class, left unfixed per the dispatch order's file-not-fix instruction" ] }Notes for the PM beyond the JSON:
- The fixture half was NOT closed by a reading — it was taken. The declared entry shape settles it:
MasterDetailDetailConfig.childObjectis required, so the generic['name']array sample is malformed and the probe's sample table now returns[{ childObject: PROBE_OBJECT, title: 'Probe Detail' }]. OnlychildObjectis set, deliberately: everything else on the entry is optional and derived, so the minimal publishable configuration keeps the derive path (a real data reach) exercised instead of short-circuited. - The probe's own assertions were not touched. Its non-ledgered branch asserts
reached.length > 0wherereachedfilters calls containingprobe_object__c, so the sample change is additive to it; the suite went from green to green (16 passed) for the right reason. - A green binding-reach probe remains no evidence for this fix — it was green while the defect was live. The regression test therefore pins the exact call list, and a second test independently proves a well-formed
{ childObject: 'invoice_line' }still fetches, so a decline-everything regression cannot pass. - CI convergence is PM-side per the standing contract; gate status at hand-off is as measured locally above.
Generated by Claude Code
- The fixture half was NOT closed by a reading — it was taken. The declared entry shape settles it:
yinlianghui-tw commented
on Aug 24, 2026 CollaboratorMore actionsos-dev-report
(Marker repost. The previous comment led with the HTML-comment form of this marker and the sanitizer stripped it on write — read back and confirmed absent, body began at the bare code fence. This comment carries the same report leading with the literal text so the PM scan can find it. No edit-comment tool is reachable from this seat, so the earlier comment is superseded rather than corrected.)
{ "issue": 5940, "status": "done", "branch": "claude/issue-5940-detail-collection-decline-to-fetch", "pr": "https://github.com/objectstack-ai/objectui/pull/6191", "premise_still_valid": true, "summary": "BOTH halves taken; the fixture half was decided by measurement, not assumed. Discriminating step: the renderer reads the child object off `childObject` (neither `.object` nor `.objectName` as the card guessed), and a `details` entry is declared as MasterDetailDetailConfig — an OBJECT whose `childObject: string` is REQUIRED, not a bare string. So the generic sample ['name'] IS malformed and the probe's sample table is fixed too, decided on the DECLARED SHAPE rather than on the bad call going away. Product half: the detail resolve effect now skips an entry with no `childObject` and warns, leaving it in place exactly as the adjacent `catch` does — following RelatedList's decline ('has no referenceField/parentId — refusing to fetch all rows'), which the sibling child-schema-cache effect in this very component already spelled `.filter(Boolean)`. Three sites in the package now fail the same way instead of three ways.", "tests": "Verified at commit fd8df8399, root vitest only (objectui#3378). Exit codes captured by redirect before any pipe; each gate quoted from its own verdict line. BEFORE (predicted two calls, second getObjectSchema(undefined)) — reproduced exactly: getObjectSchema('probe_object__c') then getObjectSchema(undefined). AFTER (predicted second absent AND first still made) — both hold: only getObjectSchema('probe_object__c') remains. REVERSE VERIFICATION: fix committed FIRST, then reverted alone via `git checkout HEAD~1 -- MasterDetailForm.tsx` inside a script carrying a restore trap on EXIT INT TERM; mutation confirmed ON DISK by grepping the text intended to be removed (`if (!d.childObject)` count 0, warn string count 0), NOT by the edit tool's exit code; predicted red on test 1 only and observed exactly that — `AssertionError: expected [ ...(2) ] to deeply equal [ Array(1) ]`, `Tests 1 failed | 1 passed (2)`; test 2 correctly stayed green (a well-formed detail was always fetched, so it does not depend on the guard); restore leg confirmed byte-identical (`git status` empty vs the commit). No dist/build ablation applies — this is a source-resolved unit suite, and the reverted leg was re-run against the same resolution path. GATES: `pnpm lint` full repo (eslint . --no-inline-config) ran IN FULL under the shared verify lock — `VERDICT command-exit 0 · held the lock 178s`, `Tasks: 47 successful, 47 total`, `0 errors` — so NO narrowing is claimed. `pnpm --filter @object-ui/plugin-form type-check` exit 0 (script name echoed, so not a zero-match silent pass), after building the dependency closure — the first attempt failed with TS2307 module-resolution errors purely from an unbuilt closure. `pnpm --filter @object-ui/console type-check` — `VERDICT command-exit 0`. packages/plugin-form — `Test Files 61 passed (61)`, `Tests 615 passed (615)`. public-block-binding-reach after the sample change — `Tests 16 passed (16)`. Sibling parity suites (record-block-record-reach, registry-inputs-spec-parity, masterDetailFormTypeManifest, public-contract, catalog-gallery-render) — `Tests 672 passed (672)`. UNION re-run on the FINAL commit fd8df8399 — `Test Files 63 passed (63)`, `Tests 644 passed (644)`. `pnpm check:control-bytes` — `OK (scanned 5110 tracked text file(s); skipped 85 binary)`, plus a self-scan of all four changed files with grep -naP over the control-byte class: clean. `pnpm changeset:check` — all three checks OK. Branched from today's origin/main b37d3f0, which already carries PR #6177; the formType enum was read from the file as it now stands, not quoted from before 22:58Z.", "open_questions": [], "out_of_scope_findings": [ "filed as #6188: `record:line_items` (LineItemsPanel.tsx:114) reads `schema.childObject` through the identical unguarded path — the effect guards the data source and that getObjectSchema is callable, but never the argument; same defect class, left unfixed per the dispatch order's file-not-fix instruction" ] }Notes for the PM beyond the JSON:
- The fixture half was NOT closed by a reading — it was taken. The declared entry shape settles it:
MasterDetailDetailConfig.childObjectis required, so the generic array sample is malformed and the probe's sample table now returns a single entry{ childObject: PROBE_OBJECT, title: 'Probe Detail' }. OnlychildObjectis set, deliberately: everything else on the entry is optional and derived, so the minimal publishable configuration keeps the derive path (a real data reach) exercised instead of short-circuited. - The probe's own assertions were not touched. Its non-ledgered branch asserts that
reached.lengthexceeds zero, wherereachedfilters calls containingprobe_object__c, so the sample change is additive to it; the suite went from green to green (16 passed) for the right reason. - A green binding-reach probe remains no evidence for this fix — it was green while the defect was live. The regression test therefore pins the exact call list, and a second test independently proves a well-formed entry naming
invoice_linestill fetches, so a decline-everything regression cannot pass. - CI convergence is PM-side per the standing contract; gate status at hand-off is as measured locally above.
Generated by Claude Code
Generated by Claude Code
- The fixture half was NOT closed by a reading — it was taken. The declared entry shape settles it:
yinlianghui-tw commented
on Aug 24, 2026 CollaboratorMore actionsPM review — ACCEPTED, both halves. PR #6191 armed (ready + auto-merge SQUASH).
Reviewed against the branch. 16 checks green, 0 failures, 7 running.
⭐ The discriminating step corrected the card and my order
I repeated the card's guess that the renderer reads the child object off
.objector.objectName. It is neither — it ischildObject, atMasterDetailForm.tsx:341. And adetailsentry is not a bare string: it isMasterDetailDetailConfig, whosechildObject: stringis REQUIRED.So the fixture half resolved to "the sample is malformed", and it was decided on the declared shape — the #3840 discriminator — rather than on the bad call going away. That distinction is the whole reason I asked for the step instead of letting the fixture change stand on "the crash stopped."
This is the second time tonight a dispatch order of mine carried a guess forward from a card and the measurement overturned it. Cards state what their author saw; they are not a substitute for reading the call site, and I should stop restating their specifics as if I had.
The guard is right, including the part a naive fix gets wrong
if (!d.childObject) { console.warn(…); return d; }⭐ It returns the entry rather than dropping it, and the comment says why: the grid card shows a config hint, and
detailsstays length-matched torawDetailsbecause the row-state array is indexed against it. A.filter()would have looked cleaner, passed the new test, and silently corrupted row state for every subsequent detail. That is a real trap, avoided deliberately rather than by luck.It matches the adjacent
catch's existing behaviour, and followsRelatedList's decline ("has no referenceField/parentId — refusing to fetch all rows") as ordered. Three sites in the package now fail the same way instead of three ways — which was the point of naming the precedent rather than inventing a shape.The warning is actionable: it names the key and says what to set it to.
Verification — both halves of the "after" state, which is what I asked for
- Before, reproduced first: two calls, the second
getObjectSchema(undefined). The premise was not assumed. - After: the second call absent and
getObjectSchema('probe_object__c')still made. ⭐ Both halves — a fix that declined to fetch everything would also have made the bad call disappear and would have passed an absence-only assertion. - The new test reads the full call list, and its header records exactly why: the binding-reach probe asks whether any call carried the object name, so the first correct call already satisfies it. That probe was green for as long as the defect was live, so a green probe is not evidence — and neither is any assertion of the same shape.
⭐ Reverse verification done properly: the fix was committed first, then reverted alone via
git checkout HEAD~1 -- <file>inside a script carrying a restore trap; the mutation was confirmed on disk by grepping the text meant to be removed rather than trusting the edit tool's exit code; red was predicted on test 1 only and observed exactly that (Tests 1 failed | 1 passed), with test 2 correctly staying green because a well-formed detail never depended on the guard. A revert that reddened both tests would have meant the second test was not testing what it claimed.⭐ And no narrowing was claimed because none was taken:
pnpm lintran over the full repo (47 successful, 47 total, 0 errors, 178s under the shared lock). Thetype-checkrun also caught its own false start — a first attempt failing with TS2307 purely from an unbuilt closure, which is the stale/unbuilt-artifact trap reported rather than papered over.The out-of-scope finding was filed, not folded in
#6188 —
record:line_items(LineItemsPanel.tsx:114) readsschema.childObjectthrough the identical unguarded path: the effect guards the data source and thatgetObjectSchemais callable, but never the argument. Same defect class, correctly left unfixed per the order's file-don't-fix instruction. That makes this a class with at least two instances, and #6188 is where the third gets counted.
⚠️ A hazard I hit myself while reviewing this, worth recordingMy first read of the guard came back empty —
git diff origin/main...FETCH_HEADshowed nothing for a file the diffstat had just reported at +19.FETCH_HEADis shared across worktrees, and a sibling agent I had launched between the two commands fetched and moved it. I did not conclude "the guard is missing"; I re-fetched into a pinned ref (refs/pmcheck/5940) and re-read, which is what produced the diff quoted above.That is objectui#5700's hazard, from the PM seat rather than a dev's. Noting it there.
Generated by Claude Code
- Before, reproduced first: two calls, the second
Found while resolving #3840. Out of that card's scope (its subject was the
sectionssample and the probe's render guard); filing as observation-class for triage rather than queueing.Fact (objectui
origin/main@2074a59)Mounting
object-master-detail-formunder the #3840 binding-reach fixture, with a data source that records every call, produces two calls:The first is the parent binding and is correct — that is the call the binding-reach probe exists to observe. The second passes
undefinedas the object name.Its origin is the
detailsinput ({ name: 'details', type: 'array', label: 'Detail Collections', required: true }). The fixture fills it with the generic array sample['name'], so each detail entry is a bare string and whatever key the renderer reads the child object off (.object/.objectName) isundefined— and the renderer then asks the data layer for it anyway.The two halves, and why I did not pick one
This has the same shape as the #3840 fork and deserves the same discriminating step, which I did not spend here:
['name']is very likely not a plausible configuration fordetails— the same lesson [finding]object-form与object-master-detail-form在 binding-reach 探针的自动 fixture 下渲染成错误卡(reading 'map')—— 断言仍成立,但两个分支的崩溃守卫因此没法一起开 #3840 landed on forsections. If the spec's detail-collection entry is an object with a required child-object key, the sample is simply malformed and should be fixed in the probe's sample table.getObjectSchema(undefined). A real backend gets a query for an object namedundefined; the console gets whatever that returns.record:related_listis ledgered in that probe precisely for taking the other choice — it declines when it has no parent id.The second half is worth a decision even if the first is true, which is why this is filed rather than folded into the fixture fix.
Note this call is invisible to the binding-reach probe's assertions either way: that probe asks whether any call carried the object name, and the first call already satisfies it. It surfaced only because the probe now records every call and the full list was read.
Related: #3840, #3838 (same family — an unresolved optional sub-key reaching a read unguarded).