diff --git a/.changeset/read-diagnostics-union-branches.md b/.changeset/read-diagnostics-union-branches.md new file mode 100644 index 0000000000..b79814440f --- /dev/null +++ b/.changeset/read-diagnostics-union-branches.md @@ -0,0 +1,28 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): 读路径 `_diagnostics` 保留 union 分支给出的真实拒绝理由 (#5598) + +`computeMetadataDiagnostics` 给 `getMetaItems()` / `getMetaItem()` 服务出去的每份 +文档挂 `_diagnostics` 信封,模块头写明它的用途是让 Studio 渲染 validity badge、 +**内联字段错误**和治理看板。但它把 zod 的 `error.issues` 直接 `.map()` 成信封条目, +而 zod 会把一个失败 `z.union` 的**全部分支**折叠成一条顶层 issue —— `path` 是 `''`, +message 是字面量 `"Invalid input"`。`ViewMetadataSchema` 顶层本身就是 union +(`z.preprocess(stripViewConsoleDecorations, z.union([...]))`),所以库里**每一个** +有缺陷的 view 文档读出来都退化成这一条没有字段名的记录,内联字段错误无处可标。 + +这不只是"少了点信息",而是**同一份文档在两条路径上判决不一致**:#5364(PR #5596) +修好写路径之后,作者**保存**一个有缺陷的 view 能看到出错的键名,**打开**同一份已存 +在库里的文档却仍然只得到一条 `Invalid input`。 + +改法是复用而不是再抄一份策略:读路径改调同包 #5596 已落地的 +`zodIssuesToMetadataIssues`,分支选取口径(丢弃只报根部 KIND 不匹配的分支;报得最少 +的分支胜出;`unrecognized_keys` 破平局;并列全出且有上限;嵌套 union 按绝对路径递归) +由该函数**单点定义**,读写两路径按构造一致。这是同一机制的第 5 个消费者 +(#4971 / #5014 / #5341 / #5364 是前四个)。 + +对消费者是**纯增量**:union 自己那条记录仍然排在 `errors[0]`,只是后面跟上了解释它的 +分支条目,所以任何读 `errors[0]` 的既有代码读到的还是同一条。没走 union 的普通字段级 +拒绝(`path` / `message` / `code`)逐字节不变;spec 合法的文档仍然是 `{ valid: true }`, +展开不会凭空造出拒绝。 diff --git a/packages/metadata-protocol/src/metadata-diagnostics.ts b/packages/metadata-protocol/src/metadata-diagnostics.ts index 8022959bfc..15c814b24b 100644 --- a/packages/metadata-protocol/src/metadata-diagnostics.ts +++ b/packages/metadata-protocol/src/metadata-diagnostics.ts @@ -27,6 +27,11 @@ import type { z } from 'zod'; import { getMetadataTypeSchema } from '@objectstack/spec/kernel'; import type { MetadataValidationResult } from '@objectstack/spec/kernel'; import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared'; +// [#5598] The READ path's share of the #5364 expansion. `zodIssuesToMetadataIssues` +// is the ONE ranking this package speaks; the save path's 422 calls the same +// function, so a document's verdict cannot depend on whether it was being saved +// or being opened. See the note above the `.safeParse()` below. +import { zodIssuesToMetadataIssues } from './protocol.js'; /** * Re-export the canonical validation-result type so callers in this @@ -74,11 +79,25 @@ export function computeMetadataDiagnostics( return { valid: true }; } - const errors = parsed.error.issues.map((issue) => ({ - path: issue.path.map(String).join('.'), - message: issue.message, - code: issue.code as string, - })); + // [#5598] NOT `parsed.error.issues.map(…)`. Zod folds every branch of a + // failed `z.union` into ONE top-level issue whose path is `''` and whose + // message is the literal `"Invalid input"`; a plain `.map()` therefore put + // exactly that on `_diagnostics` and dropped the branch that says WHICH key + // is wrong. `ViewMetadataSchema` IS a top-level union, so EVERY stored view + // with a defect degraded to one rootless line and Studio's inline field + // errors had nothing to highlight — the read-path twin of the save-path + // defect #5364 fixed, and the fifth consumer of one mechanism (#4971, + // #5014, #5341, #5364). + // + // Reusing `zodIssuesToMetadataIssues` rather than re-deriving the policy is + // the point: branch selection (drop the branches that only mismatch a root + // KIND, fewest-issues wins, `unrecognized_keys` breaks the tie, ties all + // emitted under a cap, nested unions recursed with absolute paths) is + // defined once, so opening a broken document and saving it give the author + // the same words. The expansion is strictly additive — the union's own + // entry is still first, so any consumer reading `errors[0]` today reads the + // same entry after this change. + const errors = zodIssuesToMetadataIssues(parsed.error.issues); return { valid: false, errors }; } diff --git a/packages/metadata-protocol/src/metadata-diagnostics.union-issues.test.ts b/packages/metadata-protocol/src/metadata-diagnostics.union-issues.test.ts new file mode 100644 index 0000000000..27131868d9 --- /dev/null +++ b/packages/metadata-protocol/src/metadata-diagnostics.union-issues.test.ts @@ -0,0 +1,138 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5598 — the READ path's `_diagnostics` keeps the union branch that explains + * the rejection, exactly as the save path's 422 does. + * + * `computeMetadataDiagnostics` is the fifth consumer of one mechanism + * (#4971 spec, #5014 rest, #5341 cli, #5364 the save-path 422, this one). Zod + * folds every branch of a failed `z.union` into ONE top-level issue whose path + * is `''` and whose message is the literal `"Invalid input"`; mapping only the + * top-level issues therefore put exactly that on `_diagnostics`. Since + * `ViewMetadataSchema` IS a top-level union (`view.zod.ts` — + * `z.preprocess(stripViewConsoleDecorations, z.union([…]))`), EVERY stored view + * with a defect degraded to one rootless line, and the module's own promise of + * "inline field errors" had no field to name. + * + * The consequence was a SPLIT verdict, which is what makes this its own bug + * rather than a cosmetic one: after #5364 an author who SAVED a broken view saw + * the offending key, while the same author OPENING that same stored view saw + * `Invalid input`. These tests pin the two paths to one answer by construction — + * the read path calls `zodIssuesToMetadataIssues`, it does not re-derive the + * policy — so the ranking's own behaviour stays pinned where it is defined + * (`protocol.save-union-issues.test.ts`) and is not restated here. + */ +import { describe, expect, it } from 'vitest'; +import { getMetadataTypeSchema } from '@objectstack/spec/kernel'; +import type { z } from 'zod'; +import { zodIssuesToMetadataIssues } from './protocol.js'; +import { computeMetadataDiagnostics, decorateMetadataItem } from './metadata-diagnostics.js'; + +/** + * The exact document from #5598 (and #5364 before it): a view authored with a + * single VIEW's keys at the CONTAINER level. Every branch of the top-level + * union rejects it, which is precisely the shape that used to collapse. + */ +const brokenView = { + name: 'task_list', + object: 'task', + type: 'list', + label: 'Tasks', + columns: [{ field: 'title', summary: { type: 'sum', fieldd: 'amount' } }], +}; + +describe('#5598 computeMetadataDiagnostics — a stored view names the offending key', () => { + it('expands the union instead of serving one rootless "Invalid input"', () => { + const diag = computeMetadataDiagnostics('view', brokenView); + + expect(diag?.valid).toBe(false); + // The defect, stated as a number: this was exactly 1 before the fix. + expect(diag?.errors?.length).toBeGreaterThan(1); + + // Additive, not replacing: the union's own entry is still first, so a + // consumer reading `errors[0]` reads the same entry it always did. + expect(diag?.errors?.[0]).toEqual({ + path: '', + message: 'Invalid input', + code: 'invalid_union', + }); + + // ...and the branch behind it is the one that names keys. The #4001 + // curated prose itself is spec-owned and deliberately not pinned here — + // what this file guards is that a KEY reaches the consumer at all. + const explained = diag?.errors?.slice(1) ?? []; + expect(explained.some((e) => e.code === 'unrecognized_keys')).toBe(true); + expect(explained.some((e) => e.message.includes('`columns`'))).toBe(true); + }); + + it('serves the same verdict the save path serves — one ranking, not two', () => { + // The point of the change: reuse, not a second copy of the policy. If + // this ever diverges, the same document says two different things + // depending on whether it is being opened or being saved (#5364). + const schema = getMetadataTypeSchema('view') as z.ZodTypeAny; + const parsed = schema.safeParse(brokenView); + expect(parsed.success).toBe(false); + + const fromSharedRanking = zodIssuesToMetadataIssues( + (parsed as { error: { issues: unknown[] } }).error.issues, + ); + expect(computeMetadataDiagnostics('view', brokenView)?.errors).toEqual(fromSharedRanking); + }); + + it('reaches the Studio-facing surface — `decorateMetadataItem` carries it', () => { + const decorated = decorateMetadataItem('view', brokenView) as { + _diagnostics?: { valid: boolean; errors?: Array<{ code?: string }> }; + }; + expect(decorated._diagnostics?.valid).toBe(false); + expect(decorated._diagnostics?.errors?.length).toBeGreaterThan(1); + }); + + it('re-decorating an already-decorated item is stable — the strip still runs', () => { + // `computeMetadataDiagnostics` strips its own `_diagnostics` before + // re-validating. That strip is untouched by this change, and a document + // read twice must not accumulate a rejection of the envelope itself. + const once = decorateMetadataItem('view', brokenView); + const twice = decorateMetadataItem('view', once); + expect((twice as { _diagnostics?: unknown })._diagnostics) + .toEqual((once as { _diagnostics?: unknown })._diagnostics); + }); +}); + +describe('#5598 the entries that never went through a union are unchanged', () => { + it('a plain field-level rejection keeps its own path, message and code', () => { + const diag = computeMetadataDiagnostics('object', { + name: 'task', + label: 'Task', + fields: { title: { type: 'nosuchtype' } }, + }); + + expect(diag?.valid).toBe(false); + expect(diag?.errors?.[0]?.path).toBe('fields.title.type'); + expect(diag?.errors?.[0]?.code).toBe('invalid_value'); + }); + + it('a non-object document still gets the hand-written envelope', () => { + expect(computeMetadataDiagnostics('object', null)).toEqual({ + valid: false, + errors: [{ + path: '', + message: 'Metadata document must be a non-null object', + code: 'invalid_type', + }], + }); + }); + + it('a spec-valid view stays valid — the expansion invents no rejection', () => { + const diag = computeMetadataDiagnostics('view', { + name: 'crm_lead.all', + object: 'crm_lead', + viewKind: 'list', + config: { type: 'grid', columns: ['name'], data: { provider: 'object', object: 'crm_lead' } }, + }); + expect(diag).toEqual({ valid: true }); + }); + + it('an unregistered type is still "no opinion", not "valid"', () => { + expect(computeMetadataDiagnostics('service', { name: 'whatever' })).toBeUndefined(); + }); +});