From d29275724ef215b8034daec844baf982e160319a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 23:19:39 +0000 Subject: [PATCH] =?UTF-8?q?fix(spec):=20=E5=AF=BC=E8=88=AA=E9=A1=B9?= =?UTF-8?q?=E5=9B=9B=E6=9D=A1=E3=80=8C=E5=B1=95=E5=BC=80=E3=80=8D=E5=88=AB?= =?UTF-8?q?=E5=90=8D=E6=94=B9=E4=B8=BA=E8=B7=A8=E5=8F=98=E4=BD=93=E6=95=A3?= =?UTF-8?q?=E6=96=87=20target,=E6=B6=88=E9=99=A4=E4=BA=8C=E6=AC=A1?= =?UTF-8?q?=E6=8B=92=E7=BB=9D=20(#5555)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `expanded` 只声明在 `group` 变体上,四条别名却写在被盖进九个变体的 `NAV_ITEM_ALIASES` 共享表里。于是另外八个变体把作者指向一个它们同样拒绝的 键 —— 照做之后再次被拒,且第二次没有任何建议(ledger finding 7 的二次拒绝, 由 #4001 战役自己的修复产生)。 把四条挪进按变体拼装段:`group` 保留裸键名,另外八个用散文 target `type: 'group' (with expanded)`,与既有六条跨变体别名同形。 闸门联动(alias-integrity.test.ts): - 删除 `isPinnedExpandedDefect` ≤32 容差整段 —— 债已还清,判据 2 现在零容差; - `PROSE_ALIAS_TARGETS` 扩入第七条(白名单带陈旧检查,漏改会红)。 只改面向作者的报错文案,schema 形状不变。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D --- .../nav-expanded-alias-cross-variant.md | 27 ++++ .../spec/src/shared/alias-integrity.test.ts | 64 +++----- .../src/ui/app-nav-expanded-alias.test.ts | 143 ++++++++++++++++++ packages/spec/src/ui/app.zod.ts | 39 ++++- 4 files changed, 230 insertions(+), 43 deletions(-) create mode 100644 .changeset/nav-expanded-alias-cross-variant.md create mode 100644 packages/spec/src/ui/app-nav-expanded-alias.test.ts diff --git a/.changeset/nav-expanded-alias-cross-variant.md b/.changeset/nav-expanded-alias-cross-variant.md new file mode 100644 index 0000000000..f5ec1a435c --- /dev/null +++ b/.changeset/nav-expanded-alias-cross-variant.md @@ -0,0 +1,27 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): 导航项的四条「展开」别名不再把作者指向该变体同样拒绝的键 (#5555) + +`expanded` 只声明在 `group` 导航变体上,但 `defaultOpen` / `open` / `collapsed` / +`isOpen` 这四条别名写在 `NAV_ITEM_ALIASES` —— 一张被盖进全部九个变体的共享表。于是在 +另外八个变体上,报错把作者指向一个下一步同样会被拒的键: + +1. 写 `{ type: 'url', url: '/x', defaultOpen: true }` +2. 得到 ``Did you mean `defaultOpen` → `expanded`?`` +3. 照做改成 `expanded: true` +4. **再次被拒**,而且第二次没有任何建议 + +这正是 #4001 战役要消灭的那个失败模式(ledger finding 7 的「二次拒绝」),由该战役 +自己的修复产生。 + +四条别名已挪进**按变体拼装**的那一段:`group` 保留裸键名重定向(`expanded` 在它身上 +是真键),另外八个变体改用散文 target —— 与同一段里既有的六条跨变体别名同形: + +``` +Did you mean `defaultOpen` → `type: 'group' (with expanded)`? +``` + +**只改面向作者的报错文案,schema 形状没有变化**:接受与拒绝的键集合、类型、默认值 +全部不变,已有的元数据不受影响。`group` 上的四条别名行为也不变。 diff --git a/packages/spec/src/shared/alias-integrity.test.ts b/packages/spec/src/shared/alias-integrity.test.ts index 18a325c2c9..a4932c57f5 100644 --- a/packages/spec/src/shared/alias-integrity.test.ts +++ b/packages/spec/src/shared/alias-integrity.test.ts @@ -577,6 +577,13 @@ describe('alias integrity — every table is a true claim about its schema', () * surface — fails the target criterion and has to be argued for here. The * staleness test below is the other half: an entry nothing uses is deleted, so * this list cannot quietly outlive the tables it excuses. + * + * The seventh entry arrived by that route. #5555 fixed the defect this gate + * found — the four "start expanded" spellings redirecting to `expanded` on the + * eight variants that lack it — and the fix is precisely a demotion from bare + * key name to prose, so the tolerance that pinned it was deleted and this list + * grew by one. It is one string for four alias keys because they share a single + * answer: the key you want lives on `group`. */ const PROSE_ALIAS_TARGETS: ReadonlySet = new Set([ "type: 'object' (with objectName)", @@ -585,6 +592,7 @@ const PROSE_ALIAS_TARGETS: ReadonlySet = new Set([ "type: 'dashboard' (with dashboardName)", "type: 'report' (with reportName)", "type: 'component' (with componentRef)", + "type: 'group' (with expanded)", ]); /** The surface family the prose targets are allowed on, and nowhere else. */ @@ -593,38 +601,19 @@ const PROSE_TARGET_SURFACE = /^this `[a-z]+` navigation item$/; const isProseTarget = (surface: string, target: string): boolean => PROSE_TARGET_SURFACE.test(surface) && PROSE_ALIAS_TARGETS.has(target); -/** - * A **defect this gate found**, pinned shrink-only rather than exempted: the - * four spellings of "start expanded" that `NAV_ITEM_ALIASES` redirects to - * `expanded` (#5555). - * - * `expanded` is declared on the `group` variant alone, but the shared alias - * table is stamped into all nine, so on the other eight the redirect names a - * key that variant also rejects — ledger finding 7's second rejection, from the - * campaign built to end it. The mechanism is visible in the file: the six - * CROSS-VARIANT payload keys immediately below get prose targets - * (`type: 'object' (with objectName)`) precisely because a bare key name would - * mislead; these four are cross-variant too, and were written in the shared - * table where that was not apparent. - * - * Distinguished from {@link PROSE_ALIAS_TARGETS} on purpose. That list says - * "this is right"; this one says "this is wrong, it is 32 occurrences of one - * mistake, and the fix belongs to the schema rather than the gate" — the fix - * rewrites author-facing message text in `ui/app.zod.ts`, which #5483's - * transitional guard is explicitly not allowed to touch. - * - * Structural rather than a list of 32 strings, so the tolerance cannot widen by - * accident: only this alias key, only this target, only this surface family. - * The count below is the ratchet. +/* + * The shrink-only tolerance that used to sit here (`isPinnedExpandedDefect`, + * 32 occurrences) is GONE, not relaxed: #5555 fixed the defect it pinned. + * + * It held the four "start expanded" spellings that `NAV_ITEM_ALIASES` redirected + * to `expanded` on all nine nav variants, while `expanded` is declared on `group` + * alone — so on the other eight the redirect named a key that variant also + * rejected (ledger finding 7's second rejection, from the campaign built to end + * it). The fix moved those four into the per-variant assembly: `group` keeps the + * bare key name, the other eight answer with prose, so the seventh entry in + * PROSE_ALIAS_TARGETS above is where this debt went. Claim 2 below now judges + * the family with no tolerance at all — a re-introduction lands in `broken`. */ -const EXPANDED_CROSS_VARIANT_KEYS: ReadonlySet = new Set([ - 'defaultopen', 'open', 'collapsed', 'isopen', -]); - -const isPinnedExpandedDefect = (surface: string, written: string, target: string): boolean => - PROSE_TARGET_SURFACE.test(surface) - && target === 'expanded' - && EXPANDED_CROSS_VARIANT_KEYS.has(written); /** * Guidance filed once for a table stamped nine times, legal on two of them. @@ -738,22 +727,17 @@ describe('alias integrity — direct `strictUnknownKeyError` tables (#5483)', () // is declared but accepts nothing" is invisible until the call site // migrates (#5593). const broken: string[] = []; - const pinned: string[] = []; for (const d of DIRECT) { const known = new Set(d.options.knownKeys); for (const [written, target] of Object.entries(d.options.aliases ?? {})) { if (known.has(target) || isProseTarget(d.options.surface, target)) continue; - const report = `${directEntry(d, written, target)} — \`${target}\` is not a known key here`; - if (isPinnedExpandedDefect(d.options.surface, written, target)) pinned.push(report); - else broken.push(report); + broken.push(`${directEntry(d, written, target)} — \`${target}\` is not a known key here`); } } + // No tolerance: #5555 closed the last one (see the note above + // `VARIANT_LEGAL_GUIDANCE`), so every direct table answers this claim + // outright — including the nav family that used to carry the 32. expect(broken.sort()).toEqual([]); - // The known defect, ratcheted (#5555). Shrink-only: fixing a variant drops - // the number, and nothing else can raise it — `isPinnedExpandedDefect` is - // structural, so a new broken target anywhere (including a fifth spelling - // of "expanded" on this very family) lands in `broken` above instead. - expect(pinned.length, 'the pinned #5555 tolerance grew; it may only shrink').toBeLessThanOrEqual(32); }); it('every prose-target exemption is still load-bearing', () => { diff --git a/packages/spec/src/ui/app-nav-expanded-alias.test.ts b/packages/spec/src/ui/app-nav-expanded-alias.test.ts new file mode 100644 index 0000000000..5cdca0ce0b --- /dev/null +++ b/packages/spec/src/ui/app-nav-expanded-alias.test.ts @@ -0,0 +1,143 @@ +/** + * #5555 — "start expanded" is a CROSS-VARIANT key, and the error map now says so. + * + * `expanded` is declared on the `group` nav variant alone. The four near-miss + * spellings (`defaultOpen` / `open` / `collapsed` / `isOpen`) used to be listed + * in `NAV_ITEM_ALIASES`, the table stamped into all nine variants, so on the + * other eight an author was told to write a key that variant ALSO rejects: + * + * 1. `{ type: 'url', url: '/x', defaultOpen: true }` + * 2. → "Did you mean `defaultOpen` → `expanded`?" + * 3. author writes `expanded: true` + * 4. → rejected again, and this time with no suggestion at all + * + * That is ledger finding 7's second rejection, produced by the #4001 campaign + * built to abolish it. The fix moves the four into the per-variant assembly, so + * `group` keeps the bare key name and the other eight answer with prose, exactly + * as the six cross-variant payload keys already did. + * + * These are message-text assertions because the message IS the deliverable — + * no shape changed. The reachability half is asserted too: the destination the + * prose names has to actually parse, since naming an unreachable one is the very + * defect being fixed. + */ + +import { describe, it, expect } from 'vitest'; + +import { + ObjectNavItemSchema, + DashboardNavItemSchema, + PageNavItemSchema, + UrlNavItemSchema, + ReportNavItemSchema, + ActionNavItemSchema, + ComponentNavItemSchema, + GroupNavItemSchema, + AppSchema, +} from './app.zod'; + +/** The prose target, spelled once — `alias-integrity.test.ts` allowlists it verbatim. */ +const PROSE = "type: 'group' (with expanded)"; + +/** The four spellings, as an AUTHOR writes them (camelCase; the probe folds case). */ +const EXPANDED_SPELLINGS = ['defaultOpen', 'open', 'collapsed', 'isOpen'] as const; + +type Branch = { safeParse: (v: unknown) => { success: boolean; error?: unknown } }; + +/** Every variant that does NOT own `expanded`, with the payload its `type` requires. */ +const NON_GROUP: Array<[string, Branch, Record]> = [ + ['object', ObjectNavItemSchema, { type: 'object', objectName: 'crm_lead' }], + ['dashboard', DashboardNavItemSchema, { type: 'dashboard', dashboardName: 'sales' }], + ['page', PageNavItemSchema, { type: 'page', pageName: 'home' }], + ['url', UrlNavItemSchema, { type: 'url', url: 'https://example.com' }], + ['report', ReportNavItemSchema, { type: 'report', reportName: 'pipeline' }], + ['action', ActionNavItemSchema, { type: 'action', actionDef: { actionName: 'run_it' } }], + ['component', ComponentNavItemSchema, { type: 'component', componentRef: 'metadata:resource' }], +]; + +const BASE = { id: 'nav_probe', label: 'Probe' }; + +const messagesOf = (r: { error?: unknown }): string => + JSON.stringify((r.error as { issues?: unknown })?.issues ?? []); + +describe('#5555 — the `expanded` aliases answer "right key, wrong variant"', () => { + describe('the eight variants that do not own `expanded`', () => { + it.each(NON_GROUP)('`%s` answers all four spellings with prose, not a dead key name', (_name, schema, payload) => { + for (const written of EXPANDED_SPELLINGS) { + const r = schema.safeParse({ ...BASE, ...payload, [written]: true }); + expect(r.success).toBe(false); + const issues = messagesOf(r); + expect(issues).toContain(written); + expect(issues).toContain(PROSE); + // The regression itself: the bare-key redirect must be gone. Asserted as + // the rendered arrow rather than the word `expanded`, which legitimately + // still occurs inside the prose target. + expect(issues).not.toContain(`→ \`expanded\``); + } + }); + + it('`separator` too — reached through the real door, since its branch is module-private', () => { + // `AppSchema.navigation` is a discriminatedUnion on `type`, so the + // rejection lands on the branch actually written. Without this the ninth + // variant would go unasserted here. + const r = AppSchema.safeParse({ + name: 'probe_app', + label: 'Probe', + navigation: [{ ...BASE, type: 'separator', defaultOpen: true }], + }); + expect(r.success).toBe(false); + const issues = messagesOf(r); + expect(issues).toContain(PROSE); + expect(issues).not.toContain(`→ \`expanded\``); + }); + + it('THE DEFECT — the key the old redirect named really is rejected here (a second time, silently)', () => { + // This is why the old target was a lie, and it is still true: `expanded` + // is not a `url` key. Before the fix an author reached this state by + // FOLLOWING the suggestion; the assertion pins the fact that made the + // redirect dead, so a future reader can see the fix was not cosmetic. + const r = UrlNavItemSchema.safeParse({ ...BASE, type: 'url', url: '/x', expanded: true }); + expect(r.success).toBe(false); + const issues = messagesOf(r); + expect(issues).toContain('expanded'); + // …and the second rejection carries no suggestion, which is the part that + // left the author stuck. + expect(issues).not.toContain('Did you mean'); + }); + }); + + describe('the `group` variant, which does own `expanded`', () => { + it('still redirects all four spellings to the bare key name', () => { + for (const written of EXPANDED_SPELLINGS) { + const r = GroupNavItemSchema.safeParse({ ...BASE, type: 'group', [written]: true }); + expect(r.success).toBe(false); + const issues = messagesOf(r); + expect(issues).toContain(`→ \`expanded\``); + // The prose belongs to the other eight; pointing a `group` author at + // `type: 'group'` would be a tautology. + expect(issues).not.toContain(PROSE); + } + }); + + it('REACHABILITY — following that redirect actually parses', () => { + // The half the old aliases got wrong. A suggestion is only worth making if + // the suggested authoring passes, so this is a full-parse-green assertion: + // the claim under test is a verdict on a VALUE, not merely that `expanded` + // is a recognized key name. + const r = GroupNavItemSchema.safeParse({ ...BASE, type: 'group', expanded: true }); + expect(r.success, messagesOf(r)).toBe(true); + }); + + it('REACHABILITY — and so does following the prose from another variant', () => { + // The prose says "type: 'group' (with expanded)". An author who obeys it + // rewrites the whole item, so assert that rewrite end-to-end through + // `AppSchema` rather than trusting the sentence. + const r = AppSchema.safeParse({ + name: 'probe_app', + label: 'Probe', + navigation: [{ ...BASE, type: 'group', expanded: true, children: [] }], + }); + expect(r.success, messagesOf(r)).toBe(true); + }); + }); +}); diff --git a/packages/spec/src/ui/app.zod.ts b/packages/spec/src/ui/app.zod.ts index e146bf38e8..d332804ae6 100644 --- a/packages/spec/src/ui/app.zod.ts +++ b/packages/spec/src/ui/app.zod.ts @@ -86,15 +86,44 @@ const NAV_ITEM_ALIASES: Readonly> = { requiresobjects: 'requiresObject', badgecolor: 'badgeVariant', badgestyle: 'badgeVariant', - // Found by this very gate: the platform's own Account app declared - // `defaultOpen` on three groups (never a schema key), so all three shipped - // COLLAPSED while their author believed they opened by default. +}; + +/** + * The four spellings of "start expanded", which is a CROSS-VARIANT key and not + * a shared one — so they are assembled per variant below, not listed above. + * + * Found by the alias gate: the platform's own Account app declared `defaultOpen` + * on three groups (never a schema key), so all three shipped COLLAPSED while + * their author believed they opened by default. The redirect that fixed that is + * right only where `expanded` exists, i.e. on `group`. + * + * They lived in the shared table until #5555 measured the consequence: `expanded` + * is declared on `group` alone, so on the other eight variants the redirect named + * a key that variant ALSO rejects — the author fixed the spelling as instructed + * and was rejected a second time, with no suggestion left (ledger finding 7, from + * the #4001 campaign built to end exactly that). Below they get the same prose + * treatment as the six cross-variant payload keys, for the same reason: the key + * is spelled right, the `type` is wrong, and a bare key name cannot say so. + * + * Spelled out as two literal tables rather than derived, because the prose + * string is contract surface: `alias-integrity.test.ts`'s `PROSE_ALIAS_TARGETS` + * allowlist matches it exactly, and a reader who greps the message an author + * reported has to land here. + */ +const NAV_EXPANDED_ALIASES_ON_GROUP: Readonly> = { defaultopen: 'expanded', open: 'expanded', collapsed: 'expanded', isopen: 'expanded', }; +const NAV_EXPANDED_ALIASES_ELSEWHERE: Readonly> = { + defaultopen: 'type: \'group\' (with expanded)', + open: 'type: \'group\' (with expanded)', + collapsed: 'type: \'group\' (with expanded)', + isopen: 'type: \'group\' (with expanded)', +}; + /** Per-variant payload keys, for the error map's suggestion pool. */ const NAV_VARIANT_KEYS: Readonly> = { object: ['objectName', 'viewName', 'recordId', 'recordMode', 'filters', 'children'], @@ -127,6 +156,10 @@ const navItemUnknownKeyError = (variant: keyof typeof NAV_VARIANT_KEYS) => ...(variant !== 'dashboard' ? { dashboardname: 'type: \'dashboard\' (with dashboardName)' } : {}), ...(variant !== 'report' ? { reportname: 'type: \'report\' (with reportName)' } : {}), ...(variant !== 'component' ? { componentref: 'type: \'component\' (with componentRef)' } : {}), + // `expanded` is a cross-variant key too — it just looks shared because + // "start expanded" is a sidebar-wide idea. It exists on `group` alone, so + // only `group` may answer with the bare key name (#5555). + ...(variant !== 'group' ? NAV_EXPANDED_ALIASES_ELSEWHERE : NAV_EXPANDED_ALIASES_ON_GROUP), }, guidance: { children: