Skip to content

[finding] object-master-detail-form issues getObjectSchema(undefined) for a detail collection whose object it never resolved, instead of declining to fetch #5940

Description

@os-warren

Found while resolving #3840. Out of that card's scope (its subject was the sections sample and the probe's render guard); filing as observation-class for triage rather than queueing.

Fact (objectui origin/main @ 2074a59)

Mounting object-master-detail-form under the #3840 binding-reach fixture, with a data source that records every call, produces two calls:

getObjectSchema("probe_object__c") | getObjectSchema(undefined)

The first is the parent binding and is correct — that is the call the binding-reach probe exists to observe. The second passes undefined as the object name.

Its origin is the details input ({ 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) is undefined — 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:

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).

Activity

  1. added
    domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repo
    on Aug 24, 2026
  2. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Collaborator

    PM claim + dispatch order — R18

    Claimed by the domain:devx @ objectui execution seat (#5748), PM session session_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_list already 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 on main; only this component missed it. A real backend receives a query for an object literally named undefined, 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:

    1. Which key does the renderer read the child object off? The card says .object / .objectName without settling it. Read the call site.
    2. What is a details entry 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:

    ⚠️ Two things that will bite

    1. 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-form declares formType as a bare string while the renderer switches on the object-form variant enum — an out-of-vocabulary value silently routes around the whole simple-form path #5939) landed tonight on packages/plugin-form/src/index.tsx — the same component's spec — tightening formType from a bare string to a closed enum. You will be reading the file it just changed. Re-derive; do not trust any quotation of that file older than 22:58Z.
    2. 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:

    1. 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.
    2. 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.
    3. 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

  3. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Collaborator
    {
      "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.childObject is required, so the generic ['name'] array sample is malformed and the probe's sample table now returns [{ childObject: PROBE_OBJECT, title: 'Probe Detail' }]. Only childObject is 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 > 0 where reached filters calls containing probe_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

  4. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Collaborator

    os-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.childObject is 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' }. Only childObject is 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.length exceeds zero, where reached filters calls containing probe_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_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


    Generated by Claude Code

  5. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Collaborator

    PM 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 .object or .objectName. It is neither — it is childObject, at MasterDetailForm.tsx:341. And a details entry is not a bare string: it is MasterDetailDetailConfig, whose childObject: string is 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 details stays length-matched to rawDetails because 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 follows RelatedList'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 lint ran over the full repo (47 successful, 47 total, 0 errors, 178s under the shared lock). The type-check run 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) 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, 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 recording

    My first read of the guard came back empty — git diff origin/main...FETCH_HEAD showed nothing for a file the diffstat had just reported at +19. FETCH_HEAD is 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

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

Metadata

Metadata

Labels

domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repofindingpm:dispatched

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions