Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .changeset/planned-liveness-verdict-not-dead.md
Original file line numberDiff line numberDiff line change
@@ -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.
6 changes: 5 additions & 1 deletion packages/lint/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';
Expand Down
141 changes: 140 additions & 1 deletion packages/lint/src/lint-liveness-properties.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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[]) =>
Expand DownExpand Up@@ -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<string, unknown>) => 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/);
});
});
89 changes: 75 additions & 14 deletions packages/lint/src/lint-liveness-properties.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,13 +4,15 @@
* Build-time lint that closes the spec-liveness loop on the AUTHOR side.
*
* The liveness ledgers (`@objectstack/spec/liveness/<type>.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
Expand All@@ -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<string, unknown>;

Expand DownExpand Up@@ -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. */
Expand All@@ -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}.`,
Expand Down
Loading