diff --git a/.changeset/lint-liveness-live-elsewhere-rule-id.md b/.changeset/lint-liveness-live-elsewhere-rule-id.md new file mode 100644 index 0000000000..58799ad71e --- /dev/null +++ b/.changeset/lint-liveness-live-elsewhere-rule-id.md @@ -0,0 +1,36 @@ +--- +'@objectstack/lint': minor +--- + +`lintLivenessProperties` no longer crashes on the `live-elsewhere` verdict — and never tells an author to remove a key a sibling repo enforces + +`describe()` in `lint-liveness-properties.ts` knew three verdicts +(`experimental`, `planned`, `dead`) and threw, loudly and by design, on any +other. #13483 then shipped the ledger's fifth status — `live-elsewhere`: dead +HERE by measurement, genuinely enforced in a sibling repo — and migrated +`manifest.runtime` onto it (its enforcer is the cloud marketplace publish +gate). Nothing taught `describe()` about it, so the day any `live-elsewhere` +row opts into `authorWarn: true`, `os lint` would raise that +shipped-ledger-integrity error instead of the advisory warning the author +should get. No shipped row carries `authorWarn` today, so this was a fuse +rather than a fire. + +`describe()` now has a fourth branch. `live-elsewhere` gets its own rule id — +`liveness-live-elsewhere-property`, exported as `LIVENESS_LIVE_ELSEWHERE_PROPERTY` +beside `LIVENESS_DEAD_PROPERTY` / `LIVENESS_EXPERIMENTAL_PROPERTY` / +`LIVENESS_PLANNED_PROPERTY` and advisory-only like them — plus its own message +(`is enforced in a sibling repo, not here`) and its own default hint, which keeps +the property and points at the ledger row's `evidence` for the enforcer. It +deliberately does **not** reuse the `dead` branch: that is the #11384 lesson, +which is that verdicts imply OPPOSITE author actions, and "Remove it" is the +single most damaging sentence available about a key whose enforcement is real +and remote — deleting it tears out a live gate's input. The sentinel throw +stays for genuinely unknown statuses, with its enumeration of the known ones +updated. + +The suite gains a coverage pin derived from the shipped ledgers rather than from +a hand-written list: every distinct `status` those ledgers actually carry must be +answered by `describe()` with a rule id of its own, or (for `live`, which reaches +`describe()` only through a ledger-authoring mistake) must still fail loud. A +sixth status now fails that pin by name instead of waiting for an author to trip +the sentinel. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 9f89bdcc55..da9742d0bd 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -748,6 +748,10 @@ export { LIVENESS_DEAD_PROPERTY, LIVENESS_EXPERIMENTAL_PROPERTY, LIVENESS_PLANNED_PROPERTY, + // #14057 — the fifth ledger verdict's own rule id. `live-elsewhere` is dead + // HERE by measurement but genuinely enforced in a sibling repo, so it must + // never share the `dead` id: the two ask the author for opposite actions. + LIVENESS_LIVE_ELSEWHERE_PROPERTY, } from './lint-liveness-properties.js'; export { lintAutonumberFormats } 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 43c4029176..83baa63391 100644 --- a/packages/lint/src/lint-liveness-properties.test.ts +++ b/packages/lint/src/lint-liveness-properties.test.ts @@ -9,6 +9,9 @@ import { // source for why this ONE property is tested off the ledger. checkItemAgainstWarnMap, getNested, + // #14057 coverage seam — the statuses the shipped ledgers actually carry, so + // the coverage pin below is derived from the ledgers rather than hand-listed. + shippedLedgerStatuses, } from './lint-liveness-properties.js'; /** @@ -933,7 +936,7 @@ describe('the array fan-out, against a synthetic warn map (#10262)', () => { // 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)', () => { +describe('dead / experimental / planned / live-elsewhere verdicts are distinct, and unknown statuses fail loud (#11384, #14057)', () => { const oneEntry = (entry: Record) => new Map([['gizmo', entry]]); // ── REAL LEDGER: the three rows the card captured ────────────────────── @@ -1017,6 +1020,100 @@ describe('dead / experimental / planned verdicts are distinct, and unknown statu expect(findings[0].hint).not.toContain('Remove it'); }); + // ── #14057: `live-elsewhere`, the fifth verdict — and the one whose wrong + // branch is the most expensive. #13483 added the status to the ledger and + // migrated `manifest.runtime` onto it (dead here by measurement, enforced by + // the cloud marketplace publish gate); `describe()` was not taught it, so the + // day any such row opts into `authorWarn` the author got a CRASH instead of + // the advisory finding — and the `dead` fallthrough it replaced would have + // been worse than the crash: "Remove it" about a key that is a live gate's + // input. ──────────────────────────────────────────────────────────────── + it('SYNTHETIC: a live-elsewhere entry gets its OWN rule id and a keep-it hint — never the dead branch', () => { + const findings = checkItemAgainstWarnMap( + 'gadget', + { name: 'g1', gizmo: 'x' }, + "gadget 'g1'", + oneEntry({ status: 'live-elsewhere', authorWarn: true }), + ); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.rule).toBe('liveness-live-elsewhere-property'); + expect(f.rule).not.toBe('liveness-dead-property'); + // The message must not read as a dead verdict... + expect(f.message).toContain('is enforced in a sibling repo'); + expect(f.message).not.toContain('liveness: dead'); + expect(f.message).not.toContain('has no runtime effect'); + // ...and the default hint must point at the enforcer, not at a delete key. + expect(f.hint).not.toContain('Remove it'); + expect(f.hint.toLowerCase()).toContain('keep it'); + expect(f.hint).toContain('evidence'); + }); + + it('SYNTHETIC: live-elsewhere keeps the shared hint precedence — authorHint over note over the default', () => { + const hintOf = (entry: Record) => + checkItemAgainstWarnMap('gadget', { name: 'g1', gizmo: 'x' }, "gadget 'g1'", oneEntry(entry))[0].hint; + expect(hintOf({ status: 'live-elsewhere', authorWarn: true, authorHint: 'H', note: 'N' })).toBe('H'); + expect(hintOf({ status: 'live-elsewhere', authorWarn: true, note: 'N' })).toBe('N'); + expect(hintOf({ status: 'live-elsewhere', authorWarn: true })).toContain('sibling repo'); + }); + + // NEGATIVE CONTROL, on the REAL ledger, through the production path. The card + // is a fuse, not a fire: `shouldWarn()` gates entry to `describe()`, and the + // shipped `manifest.runtime` row does not carry `authorWarn`, so nothing + // reaches the new branch today. This pin holds that reading honest in both + // directions — if the row ever opts in, this goes red and the reviewer should + // UPDATE THIS PIN (the branch above is what makes that flip safe), never + // remove the branch. + // + // Anti-vacuity: `authorWarnedProperties('manifest')` would also be empty if + // the ledger were unreadable, so the guard is that `shippedLedgerStatuses()` + // sees `live-elsewhere` at all — the ONE row carrying it lives in that very + // file, so seeing the status proves the file was read. + it('REAL LEDGER: the live-elsewhere row exists and does NOT warn yet (the fuse, unlit)', () => { + expect(shippedLedgerStatuses().has('live-elsewhere')).toBe(true); + expect(authorWarnedProperties('manifest').has('runtime')).toBe(false); + }); + + // ── COVERAGE (#14057): describe() answers for every status the ledgers ship. + // + // Patching one status is what let this card repeat #11384 — so the pin is + // derived from the shipped rows rather than from a list somebody has to + // remember to edit. A sixth status appearing in any ledger fails HERE, by + // name, instead of waiting for an author to trip the sentinel throw. + // + // `live` is the one member that must NOT get a branch, and the source header + // says why: an entry only reaches `describe()` once `shouldWarn()` has said + // yes, 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)". A mistake in our own shipped data is exactly what the + // sentinel is for, so `live` is asserted LOUD here rather than handled. + it('COVERAGE: every status the shipped ledgers carry is answered by describe() — or is loud by design', () => { + const statuses = [...shippedLedgerStatuses()].sort(); + // Anti-vacuity: an unreadable ledger dir returns the empty set, which would + // pass every assertion below without measuring anything. + expect(statuses.length).toBeGreaterThanOrEqual(4); + expect(statuses).toContain('live-elsewhere'); + + const rulesByStatus = new Map(); + for (const status of statuses) { + const run = () => + checkItemAgainstWarnMap('gadget', { name: 'g1', gizmo: 'x' }, "gadget 'g1'", oneEntry({ status, authorWarn: true })); + if (status === 'live') { + expect(run).toThrow(/live/); + continue; + } + const findings = run(); + expect(findings, `status ${status} produced no finding`).toHaveLength(1); + expect(findings[0].rule, `status ${status} has no rule id of its own`).toMatch(/^liveness-[a-z-]+-property$/); + expect(findings[0].hint.length, `status ${status} has an empty hint`).toBeGreaterThan(0); + rulesByStatus.set(status, findings[0].rule); + } + + // #11384's lesson as an assertion: verdicts imply different author actions, + // so no two of them may share a rule id. + expect(new Set(rulesByStatus.values()).size).toBe(rulesByStatus.size); + }); + // ── 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(() => @@ -1027,6 +1124,16 @@ describe('dead / experimental / planned verdicts are distinct, and unknown statu oneEntry({ status: 'quantum', authorWarn: true }), ), ).toThrow(/quantum/); + // The message enumerates what describe() DOES know — #14057 is what happens + // when that list falls behind the branches, so pin them equal. + expect(() => + checkItemAgainstWarnMap( + 'gadget', + { name: 'g1', gizmo: 'x' }, + "gadget 'g1'", + oneEntry({ status: 'quantum', authorWarn: true }), + ), + ).toThrow(/'experimental' \| 'planned' \| 'dead' \| 'live-elsewhere'/); }); it('SYNTHETIC: a `live` row mistakenly marked authorWarn also fails LOUD rather than being graded dead', () => { diff --git a/packages/lint/src/lint-liveness-properties.ts b/packages/lint/src/lint-liveness-properties.ts index ca1a527ec8..cf0b2d045d 100644 --- a/packages/lint/src/lint-liveness-properties.ts +++ b/packages/lint/src/lint-liveness-properties.ts @@ -4,15 +4,17 @@ * 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 / 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. + * authorable metadata property as live / experimental / planned / dead / + * live-elsewhere 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`, + * `planned`, or `live-elsewhere`, 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), `live-elsewhere` says keep it because a SIBLING REPO's gate enforces it + * — 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 @@ -24,7 +26,7 @@ import { createRequire } from 'node:module'; import { dirname, join } from 'node:path'; -import { existsSync, readFileSync } from 'node:fs'; +import { existsSync, readdirSync, readFileSync } from 'node:fs'; export interface LivenessLintFinding { where: string; @@ -36,6 +38,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'; +export const LIVENESS_LIVE_ELSEWHERE_PROPERTY = 'liveness-live-elsewhere-property'; type AnyRec = Record; @@ -109,28 +112,46 @@ function isAuthored(value: unknown): boolean { } /** - * `#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. + * `#11384`. The ledger ships four 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, + * and `live-elsewhere` (#13483) means keep it because it is genuinely + * ENFORCED — just not here. 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. + * + * `#14057` is the same defect class arriving through a different door, and the + * reason the boundary below is a THROW rather than a fallthrough. #13483 added + * the fifth status to the ledger vocabulary — `live-elsewhere`: dead HERE by + * measurement, genuinely enforced in a sibling repo (first row: + * `manifest.runtime`, whose enforcer is the cloud marketplace publish gate) — + * without teaching this function about it. Falling into `dead` would have said + * "Remove it" about a key whose enforcement is real and remote, i.e. exactly + * the most wrong sentence available: deleting it tears out a live gate's + * input. That row is not deletable, so `live-elsewhere` gets its own branch and + * its own rule id, and its default hint points the author at the ledger row's + * `evidence` — the enforcer's address — rather than at a delete key. * * 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. + * "Remove it"; `planned`'s and `live-elsewhere`'s must not, because removing a + * planned or elsewhere-enforced 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. + * header states "Statuses: live | experimental | planned | dead | + * live-elsewhere" 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 five. A comment is also all it is: there + * is no importable enum to switch on, which is why the coverage pin in this + * module's test derives the vocabulary from the SHIPPED LEDGER ROWS + * ({@link shippedLedgerStatuses}) instead of from that prose. + * * 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` @@ -167,14 +188,95 @@ function describe(entry: LedgerEntry): { kind: string; rule: string; defaultHint defaultHint: 'Remove it — it is declared in the spec but not consumed at runtime.', }; } + // Deliberately adjacent to `dead`, because the two are one measurement apart + // and opposite in what they ask of the author: both are inert HERE, but a + // `live-elsewhere` key is load-bearing for a gate in another repo, so the + // `dead` branch's "Remove it" is the single most damaging sentence this + // function could emit about it (#14057 / #13483). + if (entry.status === 'live-elsewhere') { + return { + kind: 'is enforced in a sibling repo, not here (liveness: live-elsewhere)', + rule: LIVENESS_LIVE_ELSEWHERE_PROPERTY, + defaultHint: + 'Keep it — nothing reads it when the stack loads here, but a sibling repo enforces it ' + + "(for `manifest.runtime`, the cloud marketplace publish gate); see the ledger row's " + + 'evidence for the enforcer.', + }; + } + // #14057 is the second time this sentinel's prediction came true, and the id + // stays HERE rather than in the message: a runtime string reaches authors and + // generated surfaces, and none of them can resolve a tracker id + // (`check:doc-authoring`, maintainer ruling 2026-08-12). 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).', + "describe() only knows 'experimental' | 'planned' | 'dead' | 'live-elsewhere'. 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).', ); } +/** + * ── Coverage seam (#14057). Package-internal: NOT part of the published surface, + * same posture as the #10262 seam below `getNested` (exported from the MODULE + * only; `src/index.ts` re-exports neither, and this package's `exports` map + * publishes just `.` and `./runtime`). ──────────────────────────────────── + * + * Every distinct `status` string the SHIPPED ledgers actually carry, walked the + * way `loadWarnMap` walks them (top-level props plus one level of `children` — + * measured as the ledgers' full depth: 423 top-level and 285 child rows carry a + * status, zero grandchildren do). + * + * WHY THIS EXISTS AND WHY IT DERIVES FROM ROWS. #14057 is #11384 repeating + * itself through a different door: `describe()` fell behind the vocabulary when + * #13483 shipped `live-elsewhere`, and nothing failed, because the gap is + * invisible until some row with the new status also opts into `authorWarn` — + * an event that had not happened yet and may not for months. A hand-written + * list of statuses in the test would have gone stale in exactly the same way + * and for exactly the same reason (nobody edits the list they did not know + * existed), so the test walks the ledgers instead: the day a sixth status + * appears in a shipped row, the coverage pin goes red naming it, rather than + * waiting for an author to trip the sentinel throw. + * + * The vocabulary is prose, not an enum — `check-liveness.mts`'s header comment + * — so the rows are the only machine-readable statement of it in this + * package's reach. That makes this a slightly NARROWER population than the + * documented vocabulary (a status documented with zero rows is not seen), and + * deliberately so: a status no row carries cannot reach `describe()`, and the + * moment one does, this returns it. + * + * Unreadable or absent ledger ⇒ the empty set — the same silent state + * `authorWarnedProperties` returns, and the reason its caller in the test + * carries an anti-vacuity guard. + */ +export function shippedLedgerStatuses(): ReadonlySet { + const statuses = new Set(); + const dir = resolveLivenessDir(); + if (!dir) return statuses; + let files: string[]; + try { + files = readdirSync(dir); + } catch { + return statuses; + } + for (const file of files) { + if (!file.endsWith('.json')) continue; + let ledger: { props?: Record }; + try { + ledger = JSON.parse(readFileSync(join(dir, file), 'utf8')); + } catch { + continue; + } + for (const entry of Object.values(ledger?.props ?? {})) { + if (typeof entry?.status === 'string') statuses.add(entry.status); + for (const child of Object.values(entry?.children ?? {})) { + if (typeof child?.status === 'string') statuses.add(child.status); + } + } + } + return statuses; +} + /** * The property paths of `type`'s ledger that warn an author for authoring them * — i.e. exactly the set {@link lintLivenessProperties} raises a finding on for