From 634719ffa99c69802b1cb81180a82e53109d38d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 22:39:03 +0000 Subject: [PATCH] fix(app-shell): audit history reads the relationship target as `reference`, a bare string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AuditFieldDef` was the third reader of a relationship field's target object, missed by objectui#6528 (which narrowed two resolvers to the spec spelling) and objectui#6648 (which narrowed the same two to the string carrier). It declared `reference_to?: string | string[]` / `reference?: string | string[]` and read `def?.reference_to ?? def?.reference` — the legacy spelling first. Both widenings are unfounded on the documents this reader is handed. Its one caller chain is `RecordDetailView`'s History effect, which passes `objectDef.fields` from `useMetadata().objects` — object metadata documents, never ObjectUI's own view/field contract. `ObjectSchema.safeParse` (spec 17.2.0) accepts `reference: ''`, refuses `reference_to` by name, and refuses both the array and `{ object }` carriers; a structure-walking census of both trees found zero producers of either carrier at the field-def key position against 599 bare-string carriers, and `AuditFieldDef` held the only `string | string[]` declaration of either key in either tree against 43 declaring `string`. Observation-class, not a live defect: the runtime `typeof` narrowing already resolved a non-string carrier to `null`, and `normalizeSchemaReferenceKeys` stamps both snake_case spellings at the ingestion choke point — which is also why dropping the `reference_to` arm loses nothing. That choke point exists so per-consumer dual-key fallbacks cannot drift; this was one of them. Both axes are pinned in the shape objectui#6528 / objectui#6648 established, plus a `@ts-expect-error` pin for the carrier, which only `tsconfig.test.json` can measure. The `predecessors` fixture moved from `reference_to` to `reference`: the old spelling is refused by name, so it was never a document this reader could legally be handed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../audit-field-def-reference-narrow.md | 13 ++ .../__tests__/auditHistoryDisplay.test.ts | 116 +++++++++++++++++- .../src/utils/auditHistoryDisplay.ts | 56 ++++++++- 3 files changed, 180 insertions(+), 5 deletions(-) create mode 100644 .changeset/audit-field-def-reference-narrow.md diff --git a/.changeset/audit-field-def-reference-narrow.md b/.changeset/audit-field-def-reference-narrow.md new file mode 100644 index 0000000000..6a32b5c227 --- /dev/null +++ b/.changeset/audit-field-def-reference-narrow.md @@ -0,0 +1,13 @@ +--- +--- + +Narrow `AuditFieldDef`'s relationship-target read in `app-shell`'s record History +diff helpers to the spec spelling `reference`, carried as a bare `string`. + +No release: nothing published changes. `AuditFieldDef` and `auditHistoryDisplay` +are package-internal — neither is re-exported from `src/index.ts`, and the +package's `exports` map exposes only `.` and `./styles.css`, so no consumer can +reach either. Runtime behaviour is unchanged on every document that can reach the +helper: `normalizeSchemaReferenceKeys` stamps both snake_case spellings at the +metadata ingestion choke point, and the removed `string[]` carrier was already +refused at runtime by the `typeof` narrowing that stayed. diff --git a/packages/app-shell/src/utils/__tests__/auditHistoryDisplay.test.ts b/packages/app-shell/src/utils/__tests__/auditHistoryDisplay.test.ts index 0bffa90d86..39fb60b8f1 100644 --- a/packages/app-shell/src/utils/__tests__/auditHistoryDisplay.test.ts +++ b/packages/app-shell/src/utils/__tests__/auditHistoryDisplay.test.ts @@ -14,12 +14,20 @@ import { collectAuditChanges, collectLookupIds, formatAuditValue, + type AuditFieldDef, } from '../auditHistoryDisplay'; +// An OBJECT METADATA DOCUMENT's `fields` record — the exact shape +// `RecordDetailView` passes (`objectDef.fields`, from `useMetadata().objects`). +// `predecessors` spelled its target `reference_to` until objectui#6719; that +// spelling is not merely non-canonical here, it is REFUSED BY NAME by +// `ObjectSchema.safeParse` (spec 17.2.0), so the fixture was never a document +// this reader could legally be handed. Re-spelled to `reference`, which the +// same parse ACCEPTS with `multiple: true` alongside it. const fields = { plan_start: { type: 'datetime', label: '计划开始日期' }, due_date: { type: 'date', label: '截止日期' }, - predecessors: { type: 'lookup', label: '紧前计划', reference_to: 'gantt_plan', multiple: true }, + predecessors: { type: 'lookup', label: '紧前计划', reference: 'gantt_plan', multiple: true }, deps_rendered: { type: 'formula', label: '紧前依赖(渲染用)' }, helper: { type: 'text', label: '内部辅助', hidden: true }, is_locked: { type: 'boolean', label: '锁定' }, @@ -104,6 +112,112 @@ describe('collectLookupIds', () => { }); }); +/** + * objectui#6719 — the relationship-target read, pinned on the two axes + * objectui#6528 (SPELLING) and objectui#6648 (CARRIER) already pinned on the + * two resolvers they covered. This helper was the third reader of the same + * value and was missed by both. + * + * `lookupTarget` is module-private, so both of its readers are asked here: + * `collectLookupIds` (which target objects get batch-resolved) and + * `formatAuditValue` (whether a raw id is swapped for a record label). + * + * WHY REFUSAL IS THE POINT. `lookupTarget`'s only caller chain is + * `RecordDetailView`'s History effect, which feeds it `objectDef.fields` from + * `useMetadata().objects` — the metadata cache for type `'object'`, i.e. OBJECT + * METADATA DOCUMENTS, never ObjectUI's own view/field contract (that contract's + * `reference_to` lives on `DetailViewFieldSchema`, which `plugin-detail` + * translates INTO from `reference`). So every spelling and carrier below is one + * `ObjectSchema.safeParse` (spec 17.2.0) refuses on the documents that actually + * arrive here, and resolving one could only re-hide the producer that emitted + * it (AGENTS.md #0.1). + */ +describe('relationship target (objectui#6719)', () => { + const change = { field: 'plan', from: 'rec_1', to: 'rec_2' }; + const targetsOf = (def: unknown): string[] => + Array.from(collectLookupIds([change], { plan: def } as any).keys()); + + it('reads the spec spelling `reference` — the positive control', () => { + expect(targetsOf({ type: 'lookup', reference: 'gantt_plan' })).toEqual(['gantt_plan']); + }); + + /** + * CARRIER axis. `FieldSchema.reference` is `optional -> string`, and + * `ObjectSchema.safeParse` refuses both carriers below — + * `reference: ['gantt_plan']` is `invalid_type: expected string, received + * array`, `reference: { object: 'gantt_plan' }` is `received object` — while + * accepting the bare name asserted above. A structure-walking, + * key-position-aware census of both trees found ZERO producers of either + * carrier at the field-def key position, against 599 bare-string carriers. + * + * The array case is the worse of the two: taking element zero would silently + * discard the rest of a multi-target value, and no such value is declared + * anywhere — polymorphic lookup is an open, unbuilt spec gap. Deleting the + * `typeof target === 'string'` narrowing turns these RED. + */ + it.each([ + { carrier: 'array', reference: ['gantt_plan'] as unknown }, + { carrier: 'multi-element array (the discarded-rest case)', reference: ['gantt_plan', 'gantt_task'] as unknown }, + { carrier: '`{ object }`', reference: { object: 'gantt_plan' } as unknown }, + ])('refuses the $carrier carrier on `reference` — a producer emitting it is the bug', ({ reference }) => { + expect(targetsOf({ type: 'lookup', reference })).toEqual([]); + }); + + /** + * The COMPILE-TIME half of the carrier axis, and the only thing that can + * measure it: `AuditFieldDef` declared `reference?: string | string[]` — the + * ONLY `string | string[]` declaration of this key in either tree, against 43 + * that declare `string`. A runtime assertion cannot see a type widen, but + * `@ts-expect-error` is a two-way pin — an unused directive is itself an + * error (TS2578) — so re-widening the member turns this file RED under + * `tsconfig.test.json`, which is what the CI `Type Check` job runs. + */ + it('declares `reference` as `string`, so the array carrier does not typecheck', () => { + // @ts-expect-error objectui#6719 — `string[]` is a shape nothing declares and nothing emits. + const widened: AuditFieldDef = { type: 'lookup', reference: ['gantt_plan', 'gantt_task'] }; + expect(targetsOf(widened)).toEqual([]); + }); + + /** + * SPELLING axis. `reference_to ?? reference` read the LEGACY key first, ahead + * of the canonical one. `ObjectSchema.safeParse` refuses all three below BY + * NAME ("Did you mean `reference_to` → `reference`?"); `reference_to` is a + * live key only on ObjectUI's own view/field contract, a different contract + * this reader is never handed. Restoring the `reference_to` arm turns the + * first case and the partial-migration case below RED; the other two + * spellings were never read here and are pinned so the chain cannot grow back + * a second time. + */ + it.each(['reference_to', 'referenceTo', 'reference_to_object'])( + 'refuses the legacy spelling `%s` — a producer emitting it is the bug', + (spelling) => { + expect(targetsOf({ type: 'lookup', [spelling]: 'gantt_plan' })).toEqual([]); + }, + ); + + it('prefers `reference` on a partially-migrated def carrying both', () => { + expect(targetsOf({ type: 'lookup', reference: 'gantt_plan', reference_to: 'stale_legacy' })).toEqual([ + 'gantt_plan', + ]); + }); + + /** + * The second reader of `lookupTarget`. A refused spelling must not reach the + * label map either — the id stays raw, which is the same visible outcome as a + * lookup whose label query failed, and never a label read off the wrong + * target object. + */ + it('does not resolve a display label through a refused spelling', () => { + const lookupLabels = new Map([['gantt_plan', new Map([['rec_1', '甘特计划B 装配']])]]); + expect(formatAuditValue({ type: 'lookup', reference: 'gantt_plan' }, 'rec_1', { lookupLabels })).toBe( + '甘特计划B 装配', + ); + expect(formatAuditValue({ type: 'lookup', reference_to: 'gantt_plan' }, 'rec_1', { lookupLabels })).toBe( + 'rec_1', + ); + }); +}); + describe('formatAuditValue', () => { it('renders empty values as empty string', () => { expect(formatAuditValue(fields.plan_start as any, null)).toBe(''); diff --git a/packages/app-shell/src/utils/auditHistoryDisplay.ts b/packages/app-shell/src/utils/auditHistoryDisplay.ts index 4b926de98d..7e6d6ef894 100644 --- a/packages/app-shell/src/utils/auditHistoryDisplay.ts +++ b/packages/app-shell/src/utils/auditHistoryDisplay.ts @@ -28,8 +28,14 @@ export interface AuditFieldDef { label?: string; hidden?: boolean; options?: unknown[]; - reference_to?: string | string[]; - reference?: string | string[]; + /** + * Relationship target object. `string`, and only `string` — see + * {@link lookupTarget} for the census behind both the carrier and the + * spelling. `reference_to` is deliberately NOT declared here: it is a key on + * ObjectUI's own view/field contract, not on an object metadata document, + * and this interface only ever describes the latter. + */ + reference?: string; [k: string]: unknown; } @@ -115,9 +121,51 @@ export function collectAuditChanges( return out; } -/** Lookup reference target when it is a single concrete object (skip polymorphic). */ +/** + * Read a lookup/master_detail field's target object from its raw def. + * + * `reference` is the ONLY spelling this reads, and only as a bare `string` — + * both narrowings are measurements, not preferences (objectui#6719, extending + * objectui#6528's SPELLING axis and objectui#6648's CARRIER axis to this + * third reader of the same value). + * + * WHAT REACHES HERE. This helper has exactly one caller chain: + * `RecordDetailView`'s History effect, which passes `objectDef.fields` — + * `objectDef` being an entry of `useMetadata().objects`, i.e. the metadata + * cache for type `'object'` (`MetadataProvider`'s `TYPE_BY_STATE_KEY.objects`). + * That is an OBJECT METADATA DOCUMENT, never ObjectUI's own view/field + * contract. `plugin-detail` does hold defs keyed `reference_to` + * (`DetailViewFieldSchema` in `@object-ui/types` `views.zod.ts`), but it + * TRANSLATES INTO that contract from `reference` (`RecordDetailDrawer`, + * `RecordMetaFooter`) and none of it flows back into `objectDef.fields`. + * + * spelling: `reference` is what `ObjectSchema.safeParse` (spec 17.2.0) + * ACCEPTS — the positive control every zero below is measured + * against. `reference_to` it REFUSES BY NAME ("Did you mean + * `reference_to` -> `reference`?"), as it does `referenceTo` and + * `reference_to_object`. Reading `reference_to` FIRST, as the + * `reference_to ?? reference` chain here did, preferred the + * spelling the contract rejects over the one it defines. + * carrier: `FieldSchema.reference` is `optional -> string`. `safeParse` + * REFUSES `reference: ['crm_account']` (`invalid_type: expected + * string, received array`) and `reference: { object: 'crm_account' }` + * (`received object`) while ACCEPTING the bare name. A + * structure-walking, key-position-aware census of both trees found + * 599 bare-string carriers at the field-def key position and ZERO + * array or `{ object }` carriers from any producer, and + * `AuditFieldDef` was the ONLY `string | string[]` declaration of + * either key in either tree against 43 that declare `string`. + * + * Dropping the `reference_to` arm loses nothing even for a def that arrives + * spelling only the legacy key: `normalizeSchemaReferenceKeys` + * (`@object-ui/core`) runs over every `'object'` item at the app-shell + * ingestion choke point and stamps BOTH snake_case keys from whichever + * spelling arrived. Its own docs give the reason this reader must not keep a + * second copy of that tolerance — the choke point exists "so per-consumer + * dual-key fallbacks can't drift" (AGENTS.md #0.1). + */ function lookupTarget(def: AuditFieldDef | undefined): string | null { - const target = def?.reference_to ?? def?.reference; + const target = def?.reference; return typeof target === 'string' && target.length > 0 ? target : null; }