Skip to content

Commit ba8420b

Browse files
os-steveclaude
andauthored
fix(lint): planned liveness rows get their own verdict and rule id (#11527)
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 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3bfa1e6 commit ba8420b

4 files changed

Lines changed: 248 additions & 16 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
'@objectstack/lint': patch
3+
---
4+
5+
`lintLivenessProperties` no longer tells authors a `planned` property is `dead`
6+
7+
`describe()` in `lint-liveness-properties.ts` only knew two verdicts
8+
(`experimental`, everything else → `dead`), while the liveness ledger ships a
9+
third: `status: 'planned'` (declared, and a consumer is being built against
10+
it — contract-first, the opposite of `dead`). Every `planned` row fell through
11+
into the `dead` branch, so the finding's own **message** told the author to
12+
remove metadata the platform had asked them to write, while the same finding's
13+
**hint** (when the row carried one) said the opposite one sentence later. Three
14+
shipped rows hit this: `field.relatedListFilter`, `object.externalSharingModel`,
15+
`translation.flows`.
16+
17+
`describe()` now has a third branch: `status === 'planned'` gets its own rule
18+
id (`liveness-planned-property`, mirroring `liveness-dead-property` /
19+
`liveness-experimental-property`'s advisory-only posture — nothing downstream
20+
keys off these ids today) and its own message/default hint ("keep it — a
21+
consumer is being built against this property", never "Remove it").
22+
23+
The ledger's `status` field is a documented vocabulary, not a Zod-enforced
24+
enum — nothing rejects a ledger entry with an unrecognised status. `describe()`
25+
previously graded any such entry `dead` silently; it now throws, naming the
26+
offending status, so a ledger-authoring mistake (a typo, or a new status added
27+
without teaching this file about it) fails loudly at test time instead of
28+
mislabelling a finding.

packages/lint/src/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,11 @@ export {
616616

617617
export { lintLivenessProperties } from './lint-liveness-properties.js';
618618
export type { LivenessLintFinding } from './lint-liveness-properties.js';
619-
export { LIVENESS_DEAD_PROPERTY, LIVENESS_EXPERIMENTAL_PROPERTY } from './lint-liveness-properties.js';
619+
export {
620+
LIVENESS_DEAD_PROPERTY,
621+
LIVENESS_EXPERIMENTAL_PROPERTY,
622+
LIVENESS_PLANNED_PROPERTY,
623+
} from './lint-liveness-properties.js';
620624

621625
export { lintAutonumberFormats } from './lint-autonumber-formats.js';
622626
export type { AutonumberLintFinding } from './lint-autonumber-formats.js';

packages/lint/src/lint-liveness-properties.test.ts

Lines changed: 140 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,8 +783,14 @@ describe('lintLivenessProperties', () => {
783783
// assertions say nothing about which properties the ledger warns on — that
784784
// stays the job of every other block in this file.
785785
describe('the array fan-out, against a synthetic warn map (#10262)', () => {
786+
// #11384: `status: 'dead'` is explicit on purpose. Before that fix, `describe()`
787+
// graded any non-`experimental` entry `dead` by fallthrough, so a synthetic
788+
// entry with no `status` at all worked here by accident; now an entry that
789+
// does not name a recognised status throws (the loud boundary the card asked
790+
// for), so this walker-only fixture must declare one — `dead` is arbitrary,
791+
// this block asserts nothing about verdicts, only about the fan-out.
786792
const warnOn = (...paths: string[]) =>
787-
new Map(paths.map((p) => [p, { authorWarn: true, authorHint: 'synthetic (#10262)' }] as const));
793+
new Map(paths.map((p) => [p, { status: 'dead', authorWarn: true, authorHint: 'synthetic (#10262)' }] as const));
788794

789795
/** `n` navigation entries; those at `authored` set the warned key. */
790796
const navItems = (n: number, authored: number[]) =>
@@ -860,3 +866,136 @@ describe('the array fan-out, against a synthetic warn map (#10262)', () => {
860866
});
861867
});
862868
});
869+
870+
// ── #11384: `describe()` gives `dead` / `experimental` / `planned` DISTINCT
871+
// verdicts — own rule id, own message, own default hint — and refuses to guess
872+
// on a status it does not recognise instead of silently grading it `dead`.
873+
//
874+
// The bug: a `planned` row (declared, and a consumer is being built against it
875+
// — the OPPOSITE of `dead`) fell through the old two-branch `describe()` into
876+
// the `dead` branch, so the finding's MESSAGE told the author to remove
877+
// something the ledger's own `authorHint`/`note` on the SAME finding said to
878+
// keep. `field.relatedListFilter`, `object.externalSharingModel` and
879+
// `translation.flows` are the three shipped rows this hit.
880+
//
881+
// The real ledgers currently have PLANNED rows and EXPERIMENTAL rows, but — as
882+
// this file's other comments document at length (#2377, #3896, #4509) — no
883+
// `dead`+`authorWarn` row survives in tree; every one that existed was retired
884+
// via enforce-or-remove rather than kept around to warn about. So the `dead`
885+
// branch, the `live`-mistakenly-warned case, and the unrecognised-status throw
886+
// are pinned here against SYNTHETIC entries through the `checkItemAgainstWarnMap`
887+
// seam (#10262) — exactly the kind of verdict-level testing that seam exists
888+
// for; the PLANNED branch is pinned against BOTH the real ledgers (so it stays
889+
// a contract test) and a synthetic no-hint entry (to pin the DEFAULT wording).
890+
describe('dead / experimental / planned verdicts are distinct, and unknown statuses fail loud (#11384)', () => {
891+
const oneEntry = (entry: Record<string, unknown>) => new Map([['gizmo', entry]]);
892+
893+
// ── REAL LEDGER: the three rows the card captured ──────────────────────
894+
it('REAL LEDGER: translation.flows (planned) — planned rule id, non-contradictory message, hint preserved', () => {
895+
const findings = lintLivenessProperties({
896+
translations: [{
897+
'zh-CN': { flows: { lead_conversion: { screens: { screen_1: { title: '转化详情' } } } } },
898+
}],
899+
});
900+
expect(findings).toHaveLength(1);
901+
const [f] = findings;
902+
expect(f.rule).toBe('liveness-planned-property');
903+
expect(f.message).not.toContain('dead');
904+
expect(f.message).toContain('is planned');
905+
// The card's own captured hint — unchanged by this fix, just no longer
906+
// contradicted by the message sitting next to it.
907+
expect(f.hint).toContain('screen-flow runner');
908+
});
909+
910+
it('REAL LEDGER: field.relatedListFilter (planned) — planned rule id, non-contradictory message', () => {
911+
const findings = lintLivenessProperties({
912+
objects: [{
913+
name: 'account',
914+
fields: [{ name: 'related_orders', type: 'text', relatedListFilter: { field: 'account_id' } }],
915+
}],
916+
});
917+
const f = findings.find((x) => x.message.includes('relatedListFilter'));
918+
expect(f).toBeDefined();
919+
expect(f!.rule).toBe('liveness-planned-property');
920+
expect(f!.message).not.toContain('dead');
921+
});
922+
923+
it('REAL LEDGER: object.externalSharingModel (planned, no authorHint — falls back to `note`) — planned rule id, note hint does not say Remove it', () => {
924+
const findings = lintLivenessProperties({ objects: [{ name: 'widget', externalSharingModel: 'read' }] });
925+
const f = findings.find((x) => x.message.includes('externalSharingModel'));
926+
expect(f).toBeDefined();
927+
expect(f!.rule).toBe('liveness-planned-property');
928+
expect(f!.message).not.toContain('dead');
929+
expect(f!.hint).not.toMatch(/^Remove it/);
930+
});
931+
932+
// ── SYNTHETIC: the default hint per verdict, when neither authorHint nor
933+
// note is present — the shape #11384 explicitly called out ("the default
934+
// hint for a planned row without an authorHint must NOT say 'Remove it'") ──
935+
it('SYNTHETIC: a planned entry with no authorHint/note gets the planned DEFAULT hint, never "Remove it"', () => {
936+
const findings = checkItemAgainstWarnMap(
937+
'gadget',
938+
{ name: 'g1', gizmo: 'x' },
939+
"gadget 'g1'",
940+
oneEntry({ status: 'planned', authorWarn: true }),
941+
);
942+
expect(findings).toHaveLength(1);
943+
expect(findings[0].rule).toBe('liveness-planned-property');
944+
expect(findings[0].message).not.toContain('dead');
945+
expect(findings[0].hint).not.toContain('Remove it');
946+
expect(findings[0].hint.toLowerCase()).toContain('keep it');
947+
});
948+
949+
it('SYNTHETIC: a dead entry with no authorHint/note keeps the dead rule id, "liveness: dead" message and the "Remove it" default hint', () => {
950+
const findings = checkItemAgainstWarnMap(
951+
'gadget',
952+
{ name: 'g1', gizmo: 'x' },
953+
"gadget 'g1'",
954+
oneEntry({ status: 'dead', authorWarn: true }),
955+
);
956+
expect(findings).toHaveLength(1);
957+
expect(findings[0].rule).toBe('liveness-dead-property');
958+
expect(findings[0].message).toContain('liveness: dead');
959+
expect(findings[0].hint).toBe('Remove it — it is declared in the spec but not consumed at runtime.');
960+
});
961+
962+
it('SYNTHETIC: an experimental entry with no authorHint/note gets an experimental default hint, never "Remove it"', () => {
963+
const findings = checkItemAgainstWarnMap(
964+
'gadget',
965+
{ name: 'g1', gizmo: 'x' },
966+
"gadget 'g1'",
967+
oneEntry({ status: 'experimental' }),
968+
);
969+
expect(findings).toHaveLength(1);
970+
expect(findings[0].rule).toBe('liveness-experimental-property');
971+
expect(findings[0].hint).not.toContain('Remove it');
972+
});
973+
974+
// ── SYNTHETIC: the unknown-status boundary — loud, never silently `dead` ──
975+
it('SYNTHETIC: an unrecognised status fails LOUD, naming the status, instead of silently grading as dead', () => {
976+
expect(() =>
977+
checkItemAgainstWarnMap(
978+
'gadget',
979+
{ name: 'g1', gizmo: 'x' },
980+
"gadget 'g1'",
981+
oneEntry({ status: 'quantum', authorWarn: true }),
982+
),
983+
).toThrow(/quantum/);
984+
});
985+
986+
it('SYNTHETIC: a `live` row mistakenly marked authorWarn also fails LOUD rather than being graded dead', () => {
987+
// Not a real shipped scenario (a `live` property should never carry
988+
// `authorWarn: true`) — but exactly the class of ledger-authoring mistake
989+
// the old silent fallthrough would have hidden by mislabelling it `dead`
990+
// too, which is why the boundary in `describe()` is status-based rather
991+
// than an `else if (status === 'planned') … else /* assume dead */`.
992+
expect(() =>
993+
checkItemAgainstWarnMap(
994+
'gadget',
995+
{ name: 'g1', gizmo: 'x' },
996+
"gadget 'g1'",
997+
oneEntry({ status: 'live', authorWarn: true }),
998+
),
999+
).toThrow(/live/);
1000+
});
1001+
});

packages/lint/src/lint-liveness-properties.ts

Lines changed: 75 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@
44
* Build-time lint that closes the spec-liveness loop on the AUTHOR side.
55
*
66
* The liveness ledgers (`@objectstack/spec/liveness/<type>.json`) classify every
7-
* authorable metadata property as live / experimental / dead with evidence. The
8-
* CI gate enforces that classification is *complete*, but the ledger's knowledge
9-
* never reached the person (very often an AI) writing the metadata. This lint
10-
* surfaces it: when an authored object/field sets a property the ledger marks as
11-
* dead-and-misleading (or experimental), it emits an advisory WARNING — "you set
12-
* this expecting it to do something; at runtime it does nothing" — with a hint
13-
* toward the supported alternative. It NEVER fails the build.
7+
* authorable metadata property as live / experimental / planned / dead with
8+
* evidence. The CI gate enforces that classification is *complete*, but the
9+
* ledger's knowledge never reached the person (very often an AI) writing the
10+
* metadata. This lint surfaces it: when an authored object/field sets a property
11+
* the ledger marks `dead`-and-misleading, `experimental`, or `planned`, it emits
12+
* an advisory WARNING with a verdict-specific message and hint — `dead` says
13+
* remove it, `experimental`/`planned` say keep it (declared, just not enforced /
14+
* not read yet) — under a verdict-specific rule id (`describe()` below is the one
15+
* place that mapping lives; #11384). It NEVER fails the build.
1416
*
1517
* Signal over noise is the whole point, so the ledger opts in per entry via
1618
* `"authorWarn": true` (+ an optional `"authorHint"`). A property being merely
@@ -33,6 +35,7 @@ export interface LivenessLintFinding {
3335

3436
export const LIVENESS_DEAD_PROPERTY = 'liveness-dead-property';
3537
export const LIVENESS_EXPERIMENTAL_PROPERTY = 'liveness-experimental-property';
38+
export const LIVENESS_PLANNED_PROPERTY = 'liveness-planned-property';
3639

3740
type AnyRec = Record<string, unknown>;
3841

@@ -105,11 +108,71 @@ function isAuthored(value: unknown): boolean {
105108
return true;
106109
}
107110

108-
function describe(entry: LedgerEntry): { kind: string; rule: string } {
111+
/**
112+
* `#11384`. The ledger ships (at least) three verdicts an author-facing finding
113+
* can carry, and they imply OPPOSITE actions: `dead` means remove the property
114+
* (nothing will ever read it), `planned` means keep it (a consumer is being
115+
* built against it, contract-first — it just does not have runtime effect
116+
* YET), `experimental` means keep it too but with the guarantee's status
117+
* flagged. Collapsing `planned` into the `dead` branch — the bug this function
118+
* fixes — told an author to delete metadata the platform had asked them to
119+
* write, while the row's own `authorHint`/`note` (when present) said the
120+
* opposite one sentence later on the SAME finding.
121+
*
122+
* Each verdict below also carries its own DEFAULT hint (used only when the
123+
* ledger entry has neither `authorHint` nor `note`): the `dead` default says
124+
* "Remove it"; `planned`'s must not, because removing a planned property is
125+
* exactly the wrong author action.
126+
*
127+
* Unknown status: `LedgerEntry.status` is a plain `string` (see the interface
128+
* above) because the ledger's status vocabulary is DOCUMENTED, not
129+
* schema-enforced — `packages/spec/scripts/liveness/check-liveness.mts`'s own
130+
* header states "Statuses: live | experimental | planned | dead" in a comment,
131+
* and nothing in that gate (or anywhere else) rejects a ledger JSON file that
132+
* spells one wrong or ships a status this function has never heard of; the
133+
* gate only requires that a status be PRESENT, not that it be one of the four.
134+
* An entry only reaches `describe()` once `shouldWarn()` has already said yes
135+
* (`authorWarn: true`, or `status === 'experimental'`), so `live` can in
136+
* principle arrive here too (an entry marked `authorWarn: true` on a `live`
137+
* row would be a ledger authoring mistake, not a user error). Before this fix
138+
* every one of those unrecognised cases fell silently into the `dead` branch —
139+
* exactly the defect class #11384 reports, just with a different trigger — so
140+
* the boundary below is LOUD on purpose: a status this function does not
141+
* recognise is a bug in the shipped ledger, not something to guess about.
142+
* This is deliberately narrower than the file's general "never throws"
143+
* promise (see the `checkItem`/bundle-walk comments below): that promise
144+
* covers malformed STACK input from an untrusted author, while a ledger
145+
* status is OUR OWN shipped, framework-controlled data — failing loudly here
146+
* cannot be triggered by anything an app author writes.
147+
*/
148+
function describe(entry: LedgerEntry): { kind: string; rule: string; defaultHint: string } {
109149
if (entry.status === 'experimental') {
110-
return { kind: 'is experimental — declared but NOT enforced at runtime', rule: LIVENESS_EXPERIMENTAL_PROPERTY };
150+
return {
151+
kind: 'is experimental — declared but NOT enforced at runtime',
152+
rule: LIVENESS_EXPERIMENTAL_PROPERTY,
153+
defaultHint: 'It is declared in the spec as an experimental guarantee — not yet enforced at runtime.',
154+
};
155+
}
156+
if (entry.status === 'planned') {
157+
return {
158+
kind: 'is planned — declared, and a consumer is being built against it (not read YET)',
159+
rule: LIVENESS_PLANNED_PROPERTY,
160+
defaultHint: 'Keep it — a consumer is being built against this property; it has no runtime effect yet.',
161+
};
162+
}
163+
if (entry.status === 'dead') {
164+
return {
165+
kind: 'has no runtime effect (liveness: dead)',
166+
rule: LIVENESS_DEAD_PROPERTY,
167+
defaultHint: 'Remove it — it is declared in the spec but not consumed at runtime.',
168+
};
111169
}
112-
return { kind: 'has no runtime effect (liveness: dead)', rule: LIVENESS_DEAD_PROPERTY };
170+
throw new Error(
171+
`lintLivenessProperties: ledger entry has unrecognised status ${JSON.stringify(entry.status)} — ` +
172+
"describe() only knows 'experimental' | 'planned' | 'dead'. This is a shipped-ledger integrity " +
173+
'bug, not an authoring error: either the ledger JSON has a typo, or a new status was added to ' +
174+
'the vocabulary without teaching describe() in lint-liveness-properties.ts about it (#11384).',
175+
);
113176
}
114177

115178
/** Check one metadata item's set properties against its type's warn-map. */
@@ -126,10 +189,8 @@ function checkItem(
126189
: [item[path]];
127190
for (const value of values instanceof Array ? values : [values]) {
128191
if (!isAuthored(value)) continue;
129-
const { kind, rule } = describe(entry);
130-
const hint = entry.authorHint
131-
?? entry.note
132-
?? 'Remove it — it is declared in the spec but not consumed at runtime.';
192+
const { kind, rule, defaultHint } = describe(entry);
193+
const hint = entry.authorHint ?? entry.note ?? defaultHint;
133194
findings.push({
134195
where: whereBase,
135196
message: `sets \`${path}\` but this ${type} property ${kind}.`,

0 commit comments

Comments
 (0)