Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .changeset/lint-primaryfield-phantom-key-removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
"@objectstack/lint": patch
---

fix(lint): 摘掉 `primaryField` 这个幽灵键——两条规则不再把它当作标题面(#6326)

`primaryField` 在 `packages/spec` 里**没有任何声明**。实测(`17.0.0-rc.5` dist):

```
ObjectSchema.safeParse({ name: 'probe_obj', label: 'Probe', primaryField: 'code',
fields: { code: { type: 'text', label: 'Code' } } })
// => success: false
// => issues: [{ code: 'unrecognized_keys', keys: ['primaryField'], path: [] }]

ObjectSchema.create(/* 同上 */)
// => throws: ObjectSchema.create('probe_obj'): unknown key(s) — primaryField.
```

同一形状换成 `nameField: 'code'` 则 `safeParse` 通过。也就是说,这个键**从来不是可声明面**,
而三处消费者没跟上——两面同源,却各自有一个可达面:

- **文档面(作者会照做,当下活着的那一半)**:`skills/objectstack-data/SKILL.md` 是 AI 编写
元数据时读的技能文档,它把 `primaryField` 明说成 `object/missing-name-field` 的合法逃逸口。
照它写出来的对象在 `ObjectSchema.create()` 上被 ADR-0032「不静默丢弃未知键」的闸硬拒——
**这是在教 AI 写出必然失败的元数据。**
- **规则面(判定永不成立)**:`data-model-rules.ts` 的 `!!obj.primaryField` 一支,以及
`validate-semantic-roles.ts` 标题解析链里的那一项,对任何 schema 收得下的对象恒为 false,
属于 #4984 家族的死支——看起来在保护什么,实际什么都判不到。

本次按维护者裁定 **remove,不 declare**(`nameField` 已是 ADR-0079 的规范主标题指针,
再立一个平行指针没有拉力,且与 Prime Directive #7「One Zod source per metadata type」相悖):

- `data-model-rules.ts`:`object/missing-name-field` 的谓词收敛为
`!!obj.nameField || fields.some(name-like)`;
- `validate-semantic-roles.ts`:规则 (d) 的标题解析链收敛为
`[nameField, displayNameField]`(`displayNameField` 实测可声明,保留);
- `skills/objectstack-data/SKILL.md`:该规则的表述改为只点名作者真正能声明的面——
`nameField` 与 name-like 字段(并列出这七个名字)。

**零 `packages/spec` 改动,不需要迁移:`primaryField` 从来不是可声明键,写了它的对象在
schema 上本来就发布不了,所以没有任何能工作的 app 会因此回归。** 行为上唯一的变化是:
一个只靠 `primaryField` 充当标题面的对象,现在会新得一条 `object/missing-name-field`
的 suggestion(severity 为 suggestion,不失败命令)——而这类对象本就通不过 `ObjectSchema`。
真正的修法是改声明 `nameField`。
73 changes: 51 additions & 22 deletions packages/cli/test/data-model-rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,23 @@ describe('lintDataModel — fields & objects', () => {
expect(has(issues, 'object/missing-name-field')).toBe(true);
});

it('accepts an object with a name field or primaryField', () => {
// #6326 replaced the second assertion here wholesale. It used to read
// `{ name: 'b', primaryField: 'code', fields: { code: … } }` and claim to pin
// the `primaryField` limb of `object/missing-name-field`. It pinned nothing,
// in two independent ways: (1) the fixture is one `ObjectSchema` REJECTS
// (`unrecognized_keys: ['primaryField']`), so no author could ever write the
// shape it was green about; and (2) `code` is itself in NAME_LIKE_FIELDS, so
// the name-like limb accepted the object regardless — the assertion stayed
// green with the `primaryField` limb deleted, which is exactly what makes it
// a vacuous green rather than coverage. What SURVIVES is the first assertion
// (a name-like field is a title face). The replacement below pins the limb
// that actually exists — the explicit `nameField` pointer — on a fixture no
// other limb can rescue, so it can genuinely fail.
it('accepts an object with a name-like field, or an explicit nameField', () => {
expect(has(lintDataModel([{ name: 'a', fields: { name: { type: 'text' } } }]), 'object/missing-name-field')).toBe(false);
expect(has(lintDataModel([{ name: 'b', primaryField: 'code', fields: { code: { type: 'text' } } }]), 'object/missing-name-field')).toBe(false);
// `invoice_number` is deliberately NOT in NAME_LIKE_FIELDS: the explicit
// ADR-0079 pointer is the only thing that can clear this object.
expect(has(lintDataModel([{ name: 'b', nameField: 'invoice_number', fields: { invoice_number: { type: 'text' } } }]), 'object/missing-name-field')).toBe(false);
});

it('handles array-shaped fields', () => {
Expand Down Expand Up @@ -186,25 +200,32 @@ describe('lintDataModel — object/missing-name-field (ADR-0079 title face)', ()
expect(has(issues, 'object/missing-name-field')).toBe(true);
});

// (c) The two untouched limbs, isolated from each other: neither fixture
// carries a field name that the other limb would also rescue.
it('leaves the primaryField and name-like limbs unchanged', () => {
// (c) #6326 — `primaryField` is NOT a title face, and the limb that read it
// is gone. The key is declared nowhere in `packages/spec`: measured on
// 17.0.0-rc.5, `ObjectSchema.safeParse` returns
// `unrecognized_keys: ['primaryField']` and `ObjectSchema.create()` throws,
// so the limb was dead for every object the spec accepts (#4984 family)
// while still reading, here and in the skill doc, as a legal escape hatch.
//
// This fixture is DELIBERATELY off-spec — that is the point. `lintDataModel`
// runs over metadata as AUTHORED, before/independently of schema parse, so a
// rejected key can physically reach it; the assertion is that the rule
// IGNORES the key, not that the key is legal. Do not "fix" the fixture by
// making it schema-valid: that would delete the coverage.
//
// Asserted as the EXACT reported set rather than as the absence of a string,
// so it fails in both directions: put the `primaryField` limb back and
// `objects[0]` drops out (set becomes `[]`); break the name-like limb and
// `objects[1]` joins it. `period_key` is not in NAME_LIKE_FIELDS and carries
// no `nameField`, so nothing else can rescue objects[0]; `crm_campaign` has
// a `name` field and must stay clean.
it('does not treat primaryField as a title face — it is not a declarable key (#6326)', () => {
expect(
has(
lintDataModel([
{ name: 'crm_forecast_period', primaryField: 'period_key', fields: { period_key: { type: 'text' } } },
]),
'object/missing-name-field',
),
).toBe(false);
expect(
has(
lintDataModel([
{ name: 'crm_campaign', fields: { name: { type: 'text' }, budget: { type: 'currency' } } },
]),
'object/missing-name-field',
),
).toBe(false);
flagged([
{ name: 'crm_forecast_period', primaryField: 'period_key', fields: { period_key: { type: 'text' } } },
{ name: 'crm_campaign', fields: { name: { type: 'text' }, budget: { type: 'currency' } } },
]),
).toEqual(['objects[0].fields']);
});

// (d) DELIBERATE FLIP, not a regression: a titleFormat-only object is now
Expand Down Expand Up @@ -241,8 +262,16 @@ describe('lintDataModel — object/missing-name-field (ADR-0079 title face)', ()
// The suggestion must name the canonical pointer — an author who reads it
// and reaches for `titleFormat` lands straight back in the contradiction.
// It must equally NOT name `primaryField`: that key is declared nowhere in
// `packages/spec`, so `ObjectSchema.create()` rejects it (#6326). The
// predicate still reads the limb; the diagnostic must not advertise it.
// `packages/spec`, so `ObjectSchema.create()` rejects it (#6326).
//
// ⚠️ Honest labelling of the `not.toContain('primaryField')` line: it is a
// NEGATIVE assertion and it was already green before #6326 (PR for #6108 had
// scrubbed the message text; #6326 only removed the predicate limb, which
// this message never mentioned). It is NOT evidence of the removal — the
// real pin for that is case (c) above. It is kept because it is paired with
// the three positive assertions below, which fail if the message stops
// steering to `nameField`/ADR-0079 at all; on its own it would be a bare
// vacuous green.
it('steers the author to nameField, and names no key the schema rejects', () => {
const issue = lintDataModel([
{ name: 'crm_quote_line_item', fields: { quantity: { type: 'number' } } },
Expand Down
27 changes: 14 additions & 13 deletions packages/lint/src/data-model-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,22 +392,23 @@ export function lintDataModel(objects: any[]): LintIssue[] {
// Reading `titleFormat` while ignoring `nameField` made this rule
// contradict its own package (#6108): an author who followed the platform's
// own migration advice earned a "records will display as raw IDs"
// suggestion, while one who kept the retired key did not. `primaryField`
// and the name-like derivation are unchanged.
// suggestion, while one who kept the retired key did not. The name-like
// derivation is unchanged.
//
// `primaryField` is kept as-is, but do NOT read it as evidence that the key
// is authorable: measured on 17.0.0-rc.5, `ObjectSchema.safeParse` reports
// `unrecognized_keys: ['primaryField']` and `ObjectSchema.create()` rejects
// it outright, so this limb can never be true for an object the spec
// accepts. Filed as #6326 (it is declared nowhere in `packages/spec`, yet
// this rule, `validate-semantic-roles` and the objectstack-data skill doc
// all treat it as a title face) — removing the limb is that issue's call,
// not a rider here. The MESSAGE, however, must not advertise it: telling an
// author to reach for `primaryField` earns them a hard schema rejection, so
// the diagnostic names only the surfaces they can actually declare.
// A third limb, `!!obj.primaryField`, was REMOVED here in #6326. That key
// is declared nowhere in `packages/spec`: measured on 17.0.0-rc.5,
// `ObjectSchema.safeParse` reports `unrecognized_keys: ['primaryField']`
// and `ObjectSchema.create()` rejects it outright, so the limb could never
// be true for an object the spec accepts — a #4984-family dead branch that
// nonetheless read as a title face here, in `validate-semantic-roles` and
// in the objectstack-data skill doc. The maintainer ruled remove, not
// declare: `nameField` is ADR-0079's one canonical title pointer and a
// second parallel pointer contradicts "one Zod source per metadata type"
// (Prime Directive #7). Do not reintroduce it as a tolerated alias — a
// consumer-side `??` for a key the producer rejects is exactly the second
// de-facto contract Prime Directive #12 bans.
const hasNameField =
!!obj.nameField ||
!!obj.primaryField ||
fields.some((f) => NAME_LIKE_FIELDS.includes(f.name));
if (fields.length > 0 && !hasNameField) {
issues.push({
Expand Down
60 changes: 60 additions & 0 deletions packages/lint/src/validate-semantic-roles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,66 @@ describe('validateSemanticRoles (ADR-0085)', () => {
expect(hiddenMember.map((f) => f.rule)).toEqual([FIELD_GROUP_SHADOWED]);
});

// #6326 — rule (d)'s title resolution used to read
// `[nameField, primaryField, displayNameField]`. `primaryField` is declared
// NOWHERE in `packages/spec`: measured on 17.0.0-rc.5,
// `ObjectSchema.safeParse` returns `unrecognized_keys: ['primaryField']` and
// `ObjectSchema.create()` throws, so the entry could never match on an object
// the spec accepts, while advertising a title pointer authors cannot write.
// `nameField` is ADR-0079's canonical one; the entry is gone.
//
// The fixture is DELIBERATELY off-spec — that is what is under test.
// `validateSemanticRoles` lints metadata as AUTHORED, so a schema-rejected
// key can physically reach it; the assertion is that it is IGNORED. Do not
// make the fixture schema-valid: that deletes the coverage.
//
// The pair below is the discriminating one. Title resolution matters here
// because the title is filtered OUT of the 4-entry strip, so whether
// `ref_no` counts as the title decides whether entry #5 (`d`) lands inside
// the strip. Read as the title → strip is [a,b,c,d], group "tail" (only
// member `d`) is fully hidden → SHADOWED. Not read → strip is
// [ref_no,a,b,c], `d` still renders → clean. No field here is one of the
// conventional fallbacks (name/full_name/title/subject/display_name), and
// none is an injected system column, so nothing else can supply a title.
it('ignores primaryField in title resolution; nameField still resolves (#6326)', () => {
const withPhantomKey = validateSemanticRoles(stack([{
name: 'ledger_entry',
primaryField: 'ref_no',
highlightFields: ['ref_no', 'a', 'b', 'c', 'd'],
fieldGroups: [{ key: 'tail', label: 'Tail' }],
fields: {
ref_no: { type: 'text' }, a: { type: 'text' }, b: { type: 'text' },
c: { type: 'text' }, d: { type: 'text', group: 'tail' },
},
}]));
// Goes red the moment the `primaryField` entry returns to the chain:
// `ref_no` would become the title, `d` would fall inside the strip, and
// one FIELD_GROUP_SHADOWED finding would appear.
expect(withPhantomKey).toEqual([]);

// Paired positive — the SAME shape with the one declarable pointer. This
// proves the assertion above is about `primaryField` being ignored, not
// about the rule being inert on this fixture.
const withNameField = validateSemanticRoles(stack([{
name: 'ledger_entry',
nameField: 'ref_no',
highlightFields: ['ref_no', 'a', 'b', 'c', 'd'],
fieldGroups: [{ key: 'tail', label: 'Tail' }],
fields: {
ref_no: { type: 'text' }, a: { type: 'text' }, b: { type: 'text' },
c: { type: 'text' }, d: { type: 'text', group: 'tail' },
},
}]));
// Indexed rather than `.map((f) => f.rule)` on purpose: this file's
// relative import of the module under test omits the `.js` extension, so
// under NodeNext every symbol it names degrades to `any` (TS2835) and each
// callback over a finding adds a TS7006 to the package's TEST_DEBT ledger.
// Same assertion strength, no new ledger entry.
expect(withNameField).toHaveLength(1);
expect(withNameField[0]?.rule).toBe(FIELD_GROUP_SHADOWED);
expect(withNameField[0]?.message).toContain('tail');
});

it('flags stageField pointing at a missing field; false is fine', () => {
const bad = validateSemanticRoles(stack([{
name: 'lead', stageField: 'pipeline', fields: { status: {} },
Expand Down
12 changes: 9 additions & 3 deletions packages/lint/src/validate-semantic-roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,15 @@ export function validateSemanticRoles(stack: AnyRec): SemanticRoleFinding[] {
);
if (declaredStrings.length > 0 && declaredGroups.size > 0) {
// Mirror the renderer's title resolution: declared role first
// (nameField / primaryField / deprecated displayNameField), else the
// first conventional display-field name present on the object.
const declaredTitle = [obj.nameField, obj.primaryField, obj.displayNameField]
// (nameField, else the deprecated displayNameField), else the first
// conventional display-field name present on the object.
//
// `primaryField` sat between those two until #6326 removed it. It is
// declared nowhere in `packages/spec` — `ObjectSchema` rejects it with
// `unrecognized_keys` — so the entry could never match on an object the
// spec accepts, and reading it here advertised a title pointer authors
// cannot write. `nameField` is ADR-0079's canonical one.
const declaredTitle = [obj.nameField, obj.displayNameField]
.find((v): v is string => typeof v === 'string' && v.length > 0 && fieldNames.has(v));
const titleField = declaredTitle
?? ['name', 'full_name', 'title', 'subject', 'display_name'].find((c) => fieldNames.has(c));
Expand Down
2 changes: 1 addition & 1 deletion skills/objectstack-data/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,7 @@ Data-model rules (in addition to naming/label/i18n):
| `relationship/association-inline-edit` | warning | an association (comment/audit/activity) marked `inlineEdit` (clutters the parent form — use a detail-page related list) |
| `rollup/missing-summary` | suggestion | a parent of numeric master_detail children with no roll-up `summary` |
| `field/select-missing-options` | warning | a `select`/`multiselect`/`radio` with no `options` (or options source) |
| `object/missing-name-field` | suggestion | an object with no name/title field or `primaryField` |
| `object/missing-name-field` | suggestion | an object with no `nameField` (ADR-0079's canonical title pointer) and no name-like field (`name`/`title`/`subject`/`label`/`full_name`/`display_name`/`code`) |

These same rules are the **rubric for AI-generated metadata** — a generation is
"good" exactly when it is schema-valid and lint-clean:
Expand Down
Loading