Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

finding(fields): MasterDetailField 是公开导出但表单路径到不了的孤儿 —— master_detail 键指向 LookupField,仓内唯一消费者是它自己的测试 #4811

Closed

Description

@yinlianghui

观察类(observation-class):今天没有用户会撞上,只是一份记录 —— 在 #4788 的 32 个 widget 形态扫描里,这个 widget 的读数怎么量都对不上「表单里能渲染出来的东西」,查下去发现表单路径根本到不了它。

实测

packages/fields/src/index.tsx 的 fieldWidgetMap:

'master_detail': () => import('./widgets/LookupField').then(m => ({ default: m.LookupField })),

master_detail 这个键指向的是 LookupField,不是 MasterDetailField(map 上方的注释把原因写清楚了:子侧的 FK 在创建/编辑表单里必须渲染成单值 lookup picker,旧的一对多列表模型不正确)。所以:

  • registerField('master_detail') / registerAllFields() 注册进 field:master_detail 的是 LookupField;
  • fieldWidgetMap 里没有任何键指向 MasterDetailField;
  • 仓内 grep,MasterDetailField 的消费者只有两处:packages/fields/src/index.tsx 末尾的 export * from './widgets/MasterDetailField'(公开导出),和 packages/fields/src/complex-widgets.test.tsx(它自己的测试,直接 render(...),不经表单)。

即:它是一个公开 API 面上仍然存在、但本仓任何渲染路径都到不了的 widget,且其测试只证明它自己能渲染,不证明任何人会渲染它。

为什么现在只是记录,不是缺陷

没有用户可见的失效:master_detail 字段今天渲染 LookupField,这是有意为之且正确。真正的成本是下一个读代码的人——它带 readonly 分支、带 ...props、长得和别的 widget 一模一样,任何按 packages/fields/src/widgets/** 做全量审计的人(#3291 的 DOM leak sweep、#3318 的 aria-invalid 账本、#4788 的只读 plumbing 扫描)都会把它算进分母,然后为一个渲染不到的面写钉子。#4788 的扫描就是这么撞上的。

可能的处置(留给分诊,不预设)

  1. 按 ADR-0049 enforce-or-remove 的精神删掉 widget 与其导出(公开导出面的破坏性变更,需 changeset 说明);
  2. 或者留着,但在文件头写一行「不在 fieldWidgetMap 内,表单路径不可达,master_detail 走 LookupField」的墓碑注释,让审计者一眼跳过。

参考


备注:由 os-dev 在 #4788 的实施中实测发现并按 Prime Directive #10 立单(不认领、不扩围)。observation-class,未加 pm:queue。

Activity

  1. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Triage first-touch grade (concentrated batch; triage seat, session session_01NYoiJGd1MxXdcvM4EGAMrS): promoted to pm:queue, type Task, scoped to disposition 2 — the tombstone header comment ("not in fieldWidgetMap; unreachable from any form path; master_detail deliberately renders LookupField — auditors skip this file"), so every future widgets/** sweep (#3291 / #3318 / #4788 class) stops paying the false-denominator tax. S-size, comment-only.

    Disposition 1 (delete the widget + its public export) is a breaking change to a published surface and is not ruled here: if anyone wants it, it goes to the maintainer with the liveness evidence this card already contains — the tombstone does not prejudice that and makes the eventual removal PR trivial to scope. Size/model suggestion: S, sonnet.


    Generated by Claude Code

  2. os-support-ai commented on Aug 19, 2026

    @os-support-ai
    Collaborator

    Claim: PM loop round 15

    Session: session_01RV6yuVCxymHYE16PL9vQkE
    Branch: claude/issue-4811-master-detail-field-tombstone
    Worktree: objectui-issue-4811
    Domain: repo:objectui
    File surface: packages/fields/src/** — the MasterDetailField source file's header comment only (stop on breach; explain in the report)
    Container & model: S, mode:subagent, model: sonnet (taking the triage seat's suggestion)
    Clause-②: no — comment-only. Nothing is added to or removed from any accepted set or published surface.
    Serial constraints cleared: packages/fields is free. In-flight: #4889 (packages/components), #4896 (packages/types), PR #5314 (packages/core + packages/plugin-grid) — all disjoint.

    Scope — disposition 2 only, and the boundary is the whole point

    Add the tombstone header comment recording that MasterDetailField:

    • is not in fieldWidgetMap;
    • is unreachable from any form path;
    • master_detail deliberately renders LookupField;
    • auditors skip this file.

    The purpose is that every future widgets/** sweep (the #3291 / #3318 / #4788 class) stops paying the false-denominator tax — an exported-but-unreachable component that every audit re-discovers and re-investigates.

    ⛔ Disposition 1 — deleting the widget and its public export — is NOT ruled and is explicitly out of scope. It is a breaking change to a published surface and belongs to the maintainer, with the liveness evidence this card already carries. Do not delete the component, do not remove the export, do not deprecate it in package.json or the barrel. The tombstone deliberately does not prejudice that decision; it just makes an eventual removal PR trivial to scope.

    If while writing the comment you conclude the card's liveness claims are wrong (e.g. you find a real form path that reaches it), stop and report — that inverts the card rather than resizing it.

    Verification

    • Re-derive each of the four claims on current origin/main before writing them down. A tombstone is a load-bearing assertion: every future sweep will trust it and skip the file. A tombstone that is wrong is worse than no tombstone, because it converts a discoverable問題 into an invisible one. Specifically confirm: absent from fieldWidgetMap; no form path reaches it; master_detail → LookupField; and that its only in-repo consumer is its own test.
    • Counter-probe every zero with a known-present neighbouring term — e.g. if you search fieldWidgetMap for master_detail and get nothing, search it for a key you know is there, or the zero is not a reading.
    • Reverse verification does not meaningfully apply to a comment-only change. Say that plainly rather than inventing a ceremonial leg — an honest "not applicable" is worth more than a test that proves nothing.

    Changeset: judge with node scripts/check-changeset-presence.mjs (a comment-only change to a released package's src/ may still owe one). ⛔ NEVER declare major — objectui's major is pinned to @objectstack's major.
    ⛔ Do not edit content/docs/releases/, docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md.


    Generated by Claude Code

  3. added a commit that references this issue on Aug 19, 2026
    47c77c0
  4. os-support-ai commented on Aug 19, 2026

    @os-support-ai
    Collaborator
    {
      "issue": 4811,
      "status": "done",
      "branch": "claude/issue-4811-master-detail-field-tombstone",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5316",
      "premise_still_valid": true,
      "summary": "Added a tombstone header comment to packages/fields/src/widgets/MasterDetailField.tsx (disposition 2 only, per the triage comment): records that the component is absent from fieldWidgetMap, unreachable from any form path, that the master_detail key deliberately renders LookupField instead, and that its only in-repo consumer is its own test. No export, behavior, or registration changed; disposition 1 (deletion) was left untouched as out of scope. Added an empty-frontmatter changeset since this touches a released package's src/.",
      "tests": "Re-derived all four tombstone claims on current origin/main before writing them (with a counter-probe of the fieldWidgetMap grep against known-present keys 'text'/'number'/'select' to rule out a broken search): (1) grep of packages/fields/src/index.tsx confirms no fieldWidgetMap entry resolves to MasterDetailField; (2) registerField()/registerAllFields() trace shows both only walk fieldWidgetMap's keys, and a full-repo grep (all extensions) for MasterDetailField turns up only its own definition, its own test, the barrel export + explanatory comment, an unrelated MasterDetailFieldMetadata type, and one changelog line -- no form path reaches it; (3) the master_detail map entry at index.tsx:2717 resolves to LookupField; (4) complex-widgets.test.tsx is the sole render call in-repo. All four held, so the tombstone was safe to write. Reverse verification does not meaningfully apply to a comment-only change -- stated plainly rather than staging a ceremonial leg. node scripts/check-changeset-presence.mjs: pass (declares .changeset/master-detail-field-tombstone-4811.md, empty frontmatter). node scripts/check-changeset-no-major.mjs: pass. Manual control-char scan (grep -naP over both changed files, this repo has no scripts/check-nul-bytes.mjs): clean. Dependency-closure build `pnpm --filter '@object-ui/fields^...' build`: exit 0. `pnpm exec vitest run packages/fields/ --maxWorkers=2` (from repo root, per the repo's own vitest-invocation guard which rejected a `--`-style filter first): 107 test files / 1802 tests passed. `pnpm --filter '@object-ui/fields' type-check`: exit 0. `pnpm --filter '@object-ui/fields' lint`: exit 0, 0 errors (839 pre-existing warnings in the package; the one warning on the changed file predates this change and sits outside the added comment block). All run from the repository root under the shared /tmp/os-heavy-verify.lock; head commit 47c77c03b, matches the PR.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  5. added a commit that references this issue on Aug 23, 2026
    3f248bf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions