From 25e144edff6c55e70c1f259ae43093568bda89d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 01:55:52 +0000 Subject: [PATCH] fix(lint): planned liveness rows get their own verdict and rule id describe() in lint-liveness-properties.ts only knew two verdicts (experimental, everything else -> dead), while the liveness ledger ships a third: status: 'planned' (declared, and a consumer is being built against it - the opposite of dead). Every planned row fell through into the dead branch, so the finding's message told the author to remove metadata the platform had asked them to write, contradicting the same finding's own hint one sentence later. Three shipped rows hit this: field.relatedListFilter, object.externalSharingModel, translation.flows. Adds a third branch (status === 'planned') with its own rule id (liveness-planned-property, mirroring the other two ids' advisory-only posture) and its own message/default hint that never says "Remove it". Also closes the silent fallthrough: an unrecognised ledger status now throws, naming the status, instead of being graded dead. Fixes #11384 --- .../planned-liveness-verdict-not-dead.md | 28 ++++ packages/lint/src/index.ts | 6 +- .../lint/src/lint-liveness-properties.test.ts | 141 +++++++++++++++++- packages/lint/src/lint-liveness-properties.ts | 89 +++++++++-- 4 files changed, 248 insertions(+), 16 deletions(-) create mode 100644 .changeset/planned-liveness-verdict-not-dead.md diff --git a/.changeset/planned-liveness-verdict-not-dead.md b/.changeset/planned-liveness-verdict-not-dead.md new file mode 100644 index 0000000000..67bd2ec761 --- /dev/null +++ b/.changeset/planned-liveness-verdict-not-dead.md @@ -0,0 +1,28 @@ +--- +'@objectstack/lint': patch +--- + +`lintLivenessProperties` no longer tells authors a `planned` property is `dead` + +`describe()` in `lint-liveness-properties.ts` only knew two verdicts +(`experimental`, everything else → `dead`), while the liveness ledger ships a +third: `status: 'planned'` (declared, and a consumer is being built against +it — contract-first, the opposite of `dead`). Every `planned` row fell through +into the `dead` branch, so the finding's own **message** told the author to +remove metadata the platform had asked them to write, while the same finding's +**hint** (when the row carried one) said the opposite one sentence later. Three +shipped rows hit this: `field.relatedListFilter`, `object.externalSharingModel`, +`translation.flows`. + +`describe()` now has a third branch: `status === 'planned'` gets its own rule +id (`liveness-planned-property`, mirroring `liveness-dead-property` / +`liveness-experimental-property`'s advisory-only posture — nothing downstream +keys off these ids today) and its own message/default hint ("keep it — a +consumer is being built against this property", never "Remove it"). + +The ledger's `status` field is a documented vocabulary, not a Zod-enforced +enum — nothing rejects a ledger entry with an unrecognised status. `describe()` +previously graded any such entry `dead` silently; it now throws, naming the +offending status, so a ledger-authoring mistake (a typo, or a new status added +without teaching this file about it) fails loudly at test time instead of +mislabelling a finding. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 0ce837204b..9ef9d27fce 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -616,7 +616,11 @@ export { export { lintLivenessProperties } from './lint-liveness-properties.js'; export type { LivenessLintFinding } from './lint-liveness-properties.js'; -export { LIVENESS_DEAD_PROPERTY, LIVENESS_EXPERIMENTAL_PROPERTY } from './lint-liveness-properties.js'; +export { + LIVENESS_DEAD_PROPERTY, + LIVENESS_EXPERIMENTAL_PROPERTY, + LIVENESS_PLANNED_PROPERTY, +} from './lint-liveness-properties.js'; export { lintAutonumberFormats } from './lint-autonumber-formats.js'; export type { AutonumberLintFinding } from './lint-autonumber-formats.js'; diff --git a/packages/lint/src/lint-liveness-properties.test.ts b/packages/lint/src/lint-liveness-properties.test.ts index 000d9f5056..9ca5e50905 100644 --- a/packages/lint/src/lint-liveness-properties.test.ts +++ b/packages/lint/src/lint-liveness-properties.test.ts @@ -783,8 +783,14 @@ describe('lintLivenessProperties', () => { // assertions say nothing about which properties the ledger warns on — that // stays the job of every other block in this file. describe('the array fan-out, against a synthetic warn map (#10262)', () => { + // #11384: `status: 'dead'` is explicit on purpose. Before that fix, `describe()` + // graded any non-`experimental` entry `dead` by fallthrough, so a synthetic + // entry with no `status` at all worked here by accident; now an entry that + // does not name a recognised status throws (the loud boundary the card asked + // for), so this walker-only fixture must declare one — `dead` is arbitrary, + // this block asserts nothing about verdicts, only about the fan-out. const warnOn = (...paths: string[]) => - new Map(paths.map((p) => [p, { authorWarn: true, authorHint: 'synthetic (#10262)' }] as const)); + new Map(paths.map((p) => [p, { status: 'dead', authorWarn: true, authorHint: 'synthetic (#10262)' }] as const)); /** `n` navigation entries; those at `authored` set the warned key. */ const navItems = (n: number, authored: number[]) => @@ -860,3 +866,136 @@ describe('the array fan-out, against a synthetic warn map (#10262)', () => { }); }); }); + +// ── #11384: `describe()` gives `dead` / `experimental` / `planned` DISTINCT +// verdicts — own rule id, own message, own default hint — and refuses to guess +// on a status it does not recognise instead of silently grading it `dead`. +// +// The bug: a `planned` row (declared, and a consumer is being built against it +// — the OPPOSITE of `dead`) fell through the old two-branch `describe()` into +// the `dead` branch, so the finding's MESSAGE told the author to remove +// something the ledger's own `authorHint`/`note` on the SAME finding said to +// keep. `field.relatedListFilter`, `object.externalSharingModel` and +// `translation.flows` are the three shipped rows this hit. +// +// The real ledgers currently have PLANNED rows and EXPERIMENTAL rows, but — as +// this file's other comments document at length (#2377, #3896, #4509) — no +// `dead`+`authorWarn` row survives in tree; every one that existed was retired +// via enforce-or-remove rather than kept around to warn about. So the `dead` +// branch, the `live`-mistakenly-warned case, and the unrecognised-status throw +// are pinned here against SYNTHETIC entries through the `checkItemAgainstWarnMap` +// seam (#10262) — exactly the kind of verdict-level testing that seam exists +// for; the PLANNED branch is pinned against BOTH the real ledgers (so it stays +// a contract test) and a synthetic no-hint entry (to pin the DEFAULT wording). +describe('dead / experimental / planned verdicts are distinct, and unknown statuses fail loud (#11384)', () => { + const oneEntry = (entry: Record) => new Map([['gizmo', entry]]); + + // ── REAL LEDGER: the three rows the card captured ────────────────────── + it('REAL LEDGER: translation.flows (planned) — planned rule id, non-contradictory message, hint preserved', () => { + const findings = lintLivenessProperties({ + translations: [{ + 'zh-CN': { flows: { lead_conversion: { screens: { screen_1: { title: '转化详情' } } } } }, + }], + }); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.rule).toBe('liveness-planned-property'); + expect(f.message).not.toContain('dead'); + expect(f.message).toContain('is planned'); + // The card's own captured hint — unchanged by this fix, just no longer + // contradicted by the message sitting next to it. + expect(f.hint).toContain('screen-flow runner'); + }); + + it('REAL LEDGER: field.relatedListFilter (planned) — planned rule id, non-contradictory message', () => { + const findings = lintLivenessProperties({ + objects: [{ + name: 'account', + fields: [{ name: 'related_orders', type: 'text', relatedListFilter: { field: 'account_id' } }], + }], + }); + const f = findings.find((x) => x.message.includes('relatedListFilter')); + expect(f).toBeDefined(); + expect(f!.rule).toBe('liveness-planned-property'); + expect(f!.message).not.toContain('dead'); + }); + + it('REAL LEDGER: object.externalSharingModel (planned, no authorHint — falls back to `note`) — planned rule id, note hint does not say Remove it', () => { + const findings = lintLivenessProperties({ objects: [{ name: 'widget', externalSharingModel: 'read' }] }); + const f = findings.find((x) => x.message.includes('externalSharingModel')); + expect(f).toBeDefined(); + expect(f!.rule).toBe('liveness-planned-property'); + expect(f!.message).not.toContain('dead'); + expect(f!.hint).not.toMatch(/^Remove it/); + }); + + // ── SYNTHETIC: the default hint per verdict, when neither authorHint nor + // note is present — the shape #11384 explicitly called out ("the default + // hint for a planned row without an authorHint must NOT say 'Remove it'") ── + it('SYNTHETIC: a planned entry with no authorHint/note gets the planned DEFAULT hint, never "Remove it"', () => { + const findings = checkItemAgainstWarnMap( + 'gadget', + { name: 'g1', gizmo: 'x' }, + "gadget 'g1'", + oneEntry({ status: 'planned', authorWarn: true }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe('liveness-planned-property'); + expect(findings[0].message).not.toContain('dead'); + expect(findings[0].hint).not.toContain('Remove it'); + expect(findings[0].hint.toLowerCase()).toContain('keep it'); + }); + + it('SYNTHETIC: a dead entry with no authorHint/note keeps the dead rule id, "liveness: dead" message and the "Remove it" default hint', () => { + const findings = checkItemAgainstWarnMap( + 'gadget', + { name: 'g1', gizmo: 'x' }, + "gadget 'g1'", + oneEntry({ status: 'dead', authorWarn: true }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe('liveness-dead-property'); + expect(findings[0].message).toContain('liveness: dead'); + expect(findings[0].hint).toBe('Remove it — it is declared in the spec but not consumed at runtime.'); + }); + + it('SYNTHETIC: an experimental entry with no authorHint/note gets an experimental default hint, never "Remove it"', () => { + const findings = checkItemAgainstWarnMap( + 'gadget', + { name: 'g1', gizmo: 'x' }, + "gadget 'g1'", + oneEntry({ status: 'experimental' }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe('liveness-experimental-property'); + expect(findings[0].hint).not.toContain('Remove it'); + }); + + // ── SYNTHETIC: the unknown-status boundary — loud, never silently `dead` ── + it('SYNTHETIC: an unrecognised status fails LOUD, naming the status, instead of silently grading as dead', () => { + expect(() => + checkItemAgainstWarnMap( + 'gadget', + { name: 'g1', gizmo: 'x' }, + "gadget 'g1'", + oneEntry({ status: 'quantum', authorWarn: true }), + ), + ).toThrow(/quantum/); + }); + + it('SYNTHETIC: a `live` row mistakenly marked authorWarn also fails LOUD rather than being graded dead', () => { + // Not a real shipped scenario (a `live` property should never carry + // `authorWarn: true`) — but exactly the class of ledger-authoring mistake + // the old silent fallthrough would have hidden by mislabelling it `dead` + // too, which is why the boundary in `describe()` is status-based rather + // than an `else if (status === 'planned') … else /* assume dead */`. + expect(() => + checkItemAgainstWarnMap( + 'gadget', + { name: 'g1', gizmo: 'x' }, + "gadget 'g1'", + oneEntry({ status: 'live', authorWarn: true }), + ), + ).toThrow(/live/); + }); +}); diff --git a/packages/lint/src/lint-liveness-properties.ts b/packages/lint/src/lint-liveness-properties.ts index 93017b20f9..44061e1f97 100644 --- a/packages/lint/src/lint-liveness-properties.ts +++ b/packages/lint/src/lint-liveness-properties.ts @@ -4,13 +4,15 @@ * Build-time lint that closes the spec-liveness loop on the AUTHOR side. * * The liveness ledgers (`@objectstack/spec/liveness/.json`) classify every - * authorable metadata property as live / experimental / dead with evidence. The - * CI gate enforces that classification is *complete*, but the ledger's knowledge - * never reached the person (very often an AI) writing the metadata. This lint - * surfaces it: when an authored object/field sets a property the ledger marks as - * dead-and-misleading (or experimental), it emits an advisory WARNING — "you set - * this expecting it to do something; at runtime it does nothing" — with a hint - * toward the supported alternative. It NEVER fails the build. + * authorable metadata property as live / experimental / planned / dead with + * evidence. The CI gate enforces that classification is *complete*, but the + * ledger's knowledge never reached the person (very often an AI) writing the + * metadata. This lint surfaces it: when an authored object/field sets a property + * the ledger marks `dead`-and-misleading, `experimental`, or `planned`, it emits + * an advisory WARNING with a verdict-specific message and hint — `dead` says + * remove it, `experimental`/`planned` say keep it (declared, just not enforced / + * not read yet) — under a verdict-specific rule id (`describe()` below is the one + * place that mapping lives; #11384). It NEVER fails the build. * * Signal over noise is the whole point, so the ledger opts in per entry via * `"authorWarn": true` (+ an optional `"authorHint"`). A property being merely @@ -33,6 +35,7 @@ export interface LivenessLintFinding { export const LIVENESS_DEAD_PROPERTY = 'liveness-dead-property'; export const LIVENESS_EXPERIMENTAL_PROPERTY = 'liveness-experimental-property'; +export const LIVENESS_PLANNED_PROPERTY = 'liveness-planned-property'; type AnyRec = Record; @@ -105,11 +108,71 @@ function isAuthored(value: unknown): boolean { return true; } -function describe(entry: LedgerEntry): { kind: string; rule: string } { +/** + * `#11384`. The ledger ships (at least) three verdicts an author-facing finding + * can carry, and they imply OPPOSITE actions: `dead` means remove the property + * (nothing will ever read it), `planned` means keep it (a consumer is being + * built against it, contract-first — it just does not have runtime effect + * YET), `experimental` means keep it too but with the guarantee's status + * flagged. Collapsing `planned` into the `dead` branch — the bug this function + * fixes — told an author to delete metadata the platform had asked them to + * write, while the row's own `authorHint`/`note` (when present) said the + * opposite one sentence later on the SAME finding. + * + * Each verdict below also carries its own DEFAULT hint (used only when the + * ledger entry has neither `authorHint` nor `note`): the `dead` default says + * "Remove it"; `planned`'s must not, because removing a planned property is + * exactly the wrong author action. + * + * Unknown status: `LedgerEntry.status` is a plain `string` (see the interface + * above) because the ledger's status vocabulary is DOCUMENTED, not + * schema-enforced — `packages/spec/scripts/liveness/check-liveness.mts`'s own + * header states "Statuses: live | experimental | planned | dead" in a comment, + * and nothing in that gate (or anywhere else) rejects a ledger JSON file that + * spells one wrong or ships a status this function has never heard of; the + * gate only requires that a status be PRESENT, not that it be one of the four. + * An entry only reaches `describe()` once `shouldWarn()` has already said yes + * (`authorWarn: true`, or `status === 'experimental'`), so `live` can in + * principle arrive here too (an entry marked `authorWarn: true` on a `live` + * row would be a ledger authoring mistake, not a user error). Before this fix + * every one of those unrecognised cases fell silently into the `dead` branch — + * exactly the defect class #11384 reports, just with a different trigger — so + * the boundary below is LOUD on purpose: a status this function does not + * recognise is a bug in the shipped ledger, not something to guess about. + * This is deliberately narrower than the file's general "never throws" + * promise (see the `checkItem`/bundle-walk comments below): that promise + * covers malformed STACK input from an untrusted author, while a ledger + * status is OUR OWN shipped, framework-controlled data — failing loudly here + * cannot be triggered by anything an app author writes. + */ +function describe(entry: LedgerEntry): { kind: string; rule: string; defaultHint: string } { if (entry.status === 'experimental') { - return { kind: 'is experimental — declared but NOT enforced at runtime', rule: LIVENESS_EXPERIMENTAL_PROPERTY }; + return { + kind: 'is experimental — declared but NOT enforced at runtime', + rule: LIVENESS_EXPERIMENTAL_PROPERTY, + defaultHint: 'It is declared in the spec as an experimental guarantee — not yet enforced at runtime.', + }; + } + if (entry.status === 'planned') { + return { + kind: 'is planned — declared, and a consumer is being built against it (not read YET)', + rule: LIVENESS_PLANNED_PROPERTY, + defaultHint: 'Keep it — a consumer is being built against this property; it has no runtime effect yet.', + }; + } + if (entry.status === 'dead') { + return { + kind: 'has no runtime effect (liveness: dead)', + rule: LIVENESS_DEAD_PROPERTY, + defaultHint: 'Remove it — it is declared in the spec but not consumed at runtime.', + }; } - return { kind: 'has no runtime effect (liveness: dead)', rule: LIVENESS_DEAD_PROPERTY }; + throw new Error( + `lintLivenessProperties: ledger entry has unrecognised status ${JSON.stringify(entry.status)} — ` + + "describe() only knows 'experimental' | 'planned' | 'dead'. This is a shipped-ledger integrity " + + 'bug, not an authoring error: either the ledger JSON has a typo, or a new status was added to ' + + 'the vocabulary without teaching describe() in lint-liveness-properties.ts about it (#11384).', + ); } /** Check one metadata item's set properties against its type's warn-map. */ @@ -126,10 +189,8 @@ function checkItem( : [item[path]]; for (const value of values instanceof Array ? values : [values]) { if (!isAuthored(value)) continue; - const { kind, rule } = describe(entry); - const hint = entry.authorHint - ?? entry.note - ?? 'Remove it — it is declared in the spec but not consumed at runtime.'; + const { kind, rule, defaultHint } = describe(entry); + const hint = entry.authorHint ?? entry.note ?? defaultHint; findings.push({ where: whereBase, message: `sets \`${path}\` but this ${type} property ${kind}.`,