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}.`,