diff --git a/.changeset/25002-page-predicate-over-budget-error.md b/.changeset/25002-page-predicate-over-budget-error.md new file mode 100644 index 00000000000..d7f4dd64430 --- /dev/null +++ b/.changeset/25002-page-predicate-over-budget-error.md @@ -0,0 +1,27 @@ +--- +'@objectstack/lint': minor +--- + +fix(lint)!: `os build`, `os validate`, `os lint` and the page save door refuse a page component props CEL slot over the platform's parse bounds (#25002) + +Clause-②: no (narrowing: a page component props CEL slot over the platform parse bounds is refused at `os build` and the page door; it ships now under the amended 「甲」, #6015 `6303865025`) + +A CEL source over `DEFAULT_LIMITS` (`maxAstNodes` 256, `maxDepth`, `maxListElements`) does not parse anywhere on the platform. It faults on every evaluation, and the console fails open, so the element it was written to gate draws for everyone. `page-predicate-over-budget` (`validatePagePredicateBounds`) finds such a source in the CEL slots declared inside a page component's `properties` bag: a `page:tabs` item `visibleWhen`, an `object-form` section or inline field `visibleWhen` / `readonlyWhen` / `requiredWhen`, `record:alert` `visible`, and the `visible` / `disabled` of the `action:*` blocks. The finding was a warning. It is now an error, and its authoring-rules entry moves from the advisory tier to the gating tier. This supersedes the warning-only behaviour described for the same rule earlier in this release line; its message no longer says "A future major release refuses it." + +**BREAKING — what moves for consumers.** + +- FROM: a warning at `os validate`, `os build` and `os lint`, and an entry on the page save door's `advisories` channel while the page saved. TO: an error. The three commands fail, and an active page write answers `422 INVALID_METADATA` with a `page-predicate-over-budget` issue located at the slot and naming the bound it overruns. The page is not stored. This covers `saveMetaItem` in publish mode (Studio, REST `/meta`, MCP) and the promotion of a draft (`publishMetaItem`). +- The page door answers `INVALID_METADATA`, its existing author-time refusal envelope, not `VALIDATION_ERROR`. + +**Fix (one line).** Shrink the predicate (collapse an `||` chain into one `in ['a', 'b', …]` test), or precompute the heavy part into a formula or roll-up field on the object and test that one field. + +**Unchanged.** + +- A within-bounds source draws nothing at any door. +- The component node's own `visibleWhen` keeps its existing verdict (`visibility-predicate-over-budget`), and this rule does not report it twice. +- Stored pages are not migrated and are not refused on read. A stored page with such a slot keeps loading and serving until it is next saved, and that save is judged. Saving a page as a draft is still allowed; publishing the draft is judged. +- `OS_ALLOW_UNLINTED_METADATA_WRITES=1` still turns a refusal into a logged write. +- No public export or signature moves; `PagePredicateBoundsFinding.severity` is now `'error'`. +- Measured before crossing, through the built `@objectstack/lint`: 0 in-repo producers carry such a slot. Every page module under `examples/` and `packages/` (30 files, 65 page values) draws 0 findings, and so do the `app-crm`, `app-multi-package` and `app-todo` composed stacks. A literal scan of 9,919 tracked source files (13,840 string literals of 120+ characters) finds 0 sources over the bounds, and HotCRM at `c8a7a70` (611 files) finds 0. A planted 78-term control is found by both instruments. + + diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index aade544ca9a..7e8fe4cc452 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -1710,14 +1710,16 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ // `record:alert` / `action:*` `visible` and `disabled`), judged for the // platform's parse bounds with `validateExpression`'s verdict. The component // node's own `visibleWhen` is the visibility family's above, an error; these - // slots were judged by no rule at any door. `advisory` in 18.x: refusing a - // page that saves today is a narrowing no ADR-0087 conversion can carry, so - // under the standing ruling for post-GA narrowings (「甲」, record - // 6292005854) only the warning ships now and the refusal waits for the next - // major. Page-local, so the per-write snapshot is the whole input. + // slots were judged by no rule at any door. `gating` since #25002: the + // finding is an `error`, so a page carrying such a slot fails all three + // commands and its next save is refused at the page door. It first shipped + // as an advisory; the amended standing ruling for post-GA narrowings + // (「甲」, record 6303865025) ships a narrowing no ADR-0087 conversion can + // carry now, as a BREAKING minor. Page-local, so the per-write snapshot is + // the whole input. { name: 'validatePagePredicateBounds', - tier: 'advisory', + tier: 'gating', input: 'normalized', commands: ALL, source: 'packages/lint/src/validate-page-predicate-bounds.ts', diff --git a/packages/lint/src/runtime-gate.page-visibility-predicate-writes.test.ts b/packages/lint/src/runtime-gate.page-visibility-predicate-writes.test.ts index 89de9d23072..155ae28fa2a 100644 --- a/packages/lint/src/runtime-gate.page-visibility-predicate-writes.test.ts +++ b/packages/lint/src/runtime-gate.page-visibility-predicate-writes.test.ts @@ -127,7 +127,7 @@ describe('#24491 — the page door gives the build\'s visibility-predicate verdi 'validateComponentTypes', 'validatePrintPageBlocks', ...FAMILY, - // #24953 — the props-bag CEL slots' bounds warning, after the family. + // #24953 / #25002 — the props-bag CEL slots' bounds refusal, after the family. 'validatePagePredicateBounds', ]); }); diff --git a/packages/lint/src/validate-page-predicate-bounds.test.ts b/packages/lint/src/validate-page-predicate-bounds.test.ts index a9b36894faf..2498a270644 100644 --- a/packages/lint/src/validate-page-predicate-bounds.test.ts +++ b/packages/lint/src/validate-page-predicate-bounds.test.ts @@ -3,11 +3,15 @@ /** * #24953 — a CEL slot inside a page component's `properties` over the * platform's parse bounds is reported, with `validateExpression`'s bounds - * verdict, at every door that accepts a page; a WARNING in 18.x (「甲」). + * verdict, at every door that accepts a page. #25002 — the finding is an + * ERROR: `os validate` / `os build` / `os lint` fail and the page door refuses + * the write (the amended 「甲」, record 6303865025). * * The card's shape is a `page:tabs` item `visibleWhen` of ~390 AST nodes, * which parsed through `PageSchema` and drew no finding anywhere while the * console faulted on it at every evaluation and drew the tab for everyone. + * The door's real save path (422, nothing stored) is pinned in + * `@objectstack/metadata-protocol`'s `protocol.runtime-authoring-gate.test.ts`. */ import { validateExpression } from '@objectstack/formula'; import { PageSchema } from '@objectstack/spec/ui'; @@ -66,15 +70,15 @@ describe('#24953 — the card\'s shape: a `page:tabs` item `visibleWhen` over `m const verdict = validateExpression('predicate', OVER_AST_NODES); expect(verdict.errors.map((e) => e.code)).toEqual(['cel-too-large']); expect(verdict.errors[0]!.params).toMatchObject({ limit: 'maxAstNodes', limitValue: 256 }); - // Parse acceptance is unchanged by this rule — it is a finding, not a refusal. + // Parse acceptance is unchanged by this rule — the refusal is the lint finding's, not the schema's. expect(PageSchema.safeParse(tabsPage(OVER_AST_NODES)).success).toBe(true); }); - it('is a WARNING naming the field and the bound, at the slot, without echoing the source', () => { + it('is an ERROR naming the field and the bound, at the slot, without echoing the source', () => { const findings = validatePagePredicateBounds({ pages: [tabsPage(OVER_AST_NODES)] }); expect(findings).toHaveLength(1); expect(findings[0]).toMatchObject({ - severity: 'warning', + severity: 'error', rule: PAGE_PREDICATE_OVER_BUDGET, where: 'page "deal_home" · page:tabs', path: TAB_PATH, @@ -82,22 +86,24 @@ describe('#24953 — the card\'s shape: a `page:tabs` item `visibleWhen` over `m expect(findings[0]!.message).toContain('`visibleWhen`'); expect(findings[0]!.message).toContain('`maxAstNodes` budget (platform limit 256)'); expect(findings[0]!.message).not.toContain('record.f0'); + // The 18.x warning's forward-looking sentence is gone: the refusal is now. + expect(findings[0]!.message).not.toContain('future major'); }); for (const command of ['validate', 'build', 'lint'] as const) { - it(`\`os ${command}\` reports it as a warning, never an error`, () => { + it(`\`os ${command}\` fails on it: an error, the only one the page draws`, () => { const stack = { pages: [tabsPage(OVER_AST_NODES)] }; const findings = runAuthoringRules(command, { normalized: stack, parsed: stack }); - expect(ours(findings).map((f) => [f.severity, f.path])).toEqual([['warning', TAB_PATH]]); - expect(findings.filter((f) => f.severity === 'error')).toEqual([]); + expect(ours(findings).map((f) => [f.severity, f.path])).toEqual([['error', TAB_PATH]]); + expect(findings.filter((f) => f.severity === 'error').map((f) => f.rule)).toEqual([PAGE_PREDICATE_OVER_BUDGET]); }); } - it('the page publish door carries it on the advisories channel, the build\'s finding verbatim', () => { + it('the page publish door refuses it on the errors channel, the build\'s finding verbatim', () => { const result = gatePage(tabsPage(OVER_AST_NODES)); expect(result.rulesRun).toContain('validatePagePredicateBounds'); - expect(ours(result.errors)).toEqual([]); - const atDoor = ours(result.advisories); + expect(ours(result.advisories)).toEqual([]); + const atDoor = ours(result.errors); expect(atDoor.map((f) => f.rule)).toEqual([PAGE_PREDICATE_OVER_BUDGET]); const stack = { pages: [tabsPage(OVER_AST_NODES)] }; const atBuild = ours(runAuthoringRules('build', { normalized: stack, parsed: stack })); @@ -186,9 +192,9 @@ describe('#24953 — no double report with the visibility family', () => { }); describe('#24953 — wiring', () => { - it('runs on all three commands as an advisory rule, and at the page door only', () => { + it('runs on all three commands as a gating rule, and at the page door only', () => { const entry = authoringRulesFor('validate').find((r) => r.name === 'validatePagePredicateBounds'); - expect(entry?.tier).toBe('advisory'); + expect(entry?.tier).toBe('gating'); for (const command of ['build', 'lint'] as const) { expect(authoringRulesFor(command).map((r) => r.name)).toContain('validatePagePredicateBounds'); } diff --git a/packages/lint/src/validate-page-predicate-bounds.ts b/packages/lint/src/validate-page-predicate-bounds.ts index 965eca646c6..e4a15923c57 100644 --- a/packages/lint/src/validate-page-predicate-bounds.ts +++ b/packages/lint/src/validate-page-predicate-bounds.ts @@ -58,14 +58,17 @@ * budget by definition, and `validateExpression`'s own `detail` quotes the * author's line); the `path` locates it. * - * ## Why a warning + * ## Why an error * - * Refusing a page that saves today is a narrowing no ADR-0087 conversion can - * carry — a predicate cannot be shrunk mechanically — so under the standing - * ruling for breaking narrowings after the 18.0 GA (「甲」, record 6292005854) - * the 18.x line ships the loud half only: a `warning` at `os validate`, - * `os build`, `os lint` and the page publish door's `advisories` channel. The - * refusal belongs to the next major, which promotes the finding to `error`. + * Such a source never gates what it was written to gate, at any door, so the + * finding is a refusal: `os validate`, `os build` and `os lint` fail, and the + * page publish door answers its structured 422 on the page's next save. + * Stored pages keep loading and serving — the gate judges writes, never the + * read path. The 18.x line first shipped this as a warning; under the amended + * standing ruling for breaking narrowings (「甲」, record 6303865025: a + * narrowing no ADR-0087 conversion can carry also ships now, as a BREAKING + * minor) the refusal ships in 18.x too. No conversion carries it: a predicate + * cannot be shrunk mechanically, so the author's fix is the hint below. */ import { validateExpression } from '@objectstack/formula'; @@ -78,7 +81,7 @@ import { recordsOf } from './object-graph.js'; export const PAGE_PREDICATE_OVER_BUDGET = 'page-predicate-over-budget'; export interface PagePredicateBoundsFinding { - severity: 'warning'; + severity: 'error'; rule: typeof PAGE_PREDICATE_OVER_BUDGET; /** e.g. `page "pipeline_home" · page:tabs`. */ where: string; @@ -202,14 +205,13 @@ export function validatePagePredicateBounds(stack: AnyRec): PagePredicateBoundsF ? `the \`${verdict.limit}\` budget (platform limit ${verdict.limitValue})` : "one of the platform's CEL parse budgets"; findings.push({ - severity: 'warning', + severity: 'error', rule: PAGE_PREDICATE_OVER_BUDGET, where: `page "${pageName}" · ${type}`, path: `${path}.properties${slot.rel}`, message: `\`${slot.key}\` is valid CEL that exceeds ${bound}, so it does not parse anywhere on the ` + - 'platform: it faults on every evaluation and never gates what it was written to gate. ' + - 'A future major release refuses it.', + 'platform: it faults on every evaluation and never gates what it was written to gate.', hint: "Make the expression smaller: collapse an `||` chain into one `in ['a', 'b', …]` test, or " + 'precompute the heavy part into a formula or roll-up field on the object and test that one field', diff --git a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts index e07d8cee31e..8bda424bdab 100644 --- a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts @@ -3471,6 +3471,116 @@ describe('runtime authoring gate on PAGE writes — the component visibleWhen ve }); }); +// #25002 — a CEL slot INSIDE a page component's `properties` bag that overruns +// the platform's parse bounds (`page-predicate-over-budget`, +// `validatePagePredicateBounds`) is refused at the runtime save door, as +// `os build` refuses it. #24953 shipped it as an advisory (the door admitted the +// save and listed the finding); the amended 「甲」 (record 6303865025) ships +// the refusal in 18.x. The card's shape: a `page:tabs` item `visibleWhen` of +// ~390 AST nodes, over `maxAstNodes` (256). Pinned through the REAL +// `saveMetaItem`: +// +// (a) the active save is refused — 422 `INVALID_METADATA`, the door's own +// envelope — naming the slot and the bound, and nothing lands; the draft +// promotion is refused the same way; +// (b) the door's finding IS the build's; +// (c) control: a within-bounds tab predicate saves, and the row lands. +// +// ⚠️ As above: this package reaches `@objectstack/lint` through its built +// `dist/`, so a registry or rule edit is invisible here until +// `pnpm --filter @objectstack/lint build` has run. +describe('runtime authoring gate on PAGE writes — a props-bag CEL slot over the parse bounds (#25002)', () => { + const RULE = 'page-predicate-over-budget'; + const TAB_PATH = 'pages[0].regions[0].components[0].properties.items[1].visibleWhen'; + /** 78 `||` terms, five AST nodes each — the card's ~390-node class. */ + const OVER = Array.from({ length: 78 }, (_, i) => `record.f${i} == ${i}`).join(' || '); + const WITHIN = "record.stage == 'won' || page.tab != ''"; + const page = (tabVisibleWhen: string) => ({ + name: 'deal_home', + label: 'Deal', + type: 'app', + regions: [{ + name: 'main', + components: [{ + type: 'page:tabs', + properties: { + items: [ + { label: 'Overview', value: 'overview', children: [] }, + { label: 'Finance', value: 'finance', visibleWhen: tabVisibleWhen, children: [] }, + ], + }, + }], + }], + }); + + const savePage = (protocol: any, item: unknown, extra: Record = {}) => + protocol.saveMetaItem({ type: 'page', name: 'deal_home', item, ...extra }); + const activePageRows = (rows: Map) => + Array.from(rows.values()).filter((r) => r.type === 'page' && r.state === 'active'); + + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + afterEach(() => { + warn.mockRestore(); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + + it('(a) REFUSES an active save of the card\'s tab-item `visibleWhen`, naming the bound; nothing stored', async () => { + const { protocol, rows } = makeProtocol(); + + const err = await savePage(protocol, page(OVER)).catch((e: any) => e); + + expect(err, 'the save resolved — the door still admits the over-budget slot').toBeInstanceOf(Error); + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect(err.rulesRun).toContain('validatePagePredicateBounds'); + const issues = (err.issues ?? []).filter((i: any) => i.rule === RULE); + expect(issues, `issues: ${JSON.stringify(err.issues)}`).toHaveLength(1); + expect(issues[0]).toMatchObject({ severity: 'error', path: TAB_PATH }); + expect(issues[0].message).toContain('`maxAstNodes` budget (platform limit 256)'); + expect(activePageRows(rows)).toEqual([]); + }); + + it('(a) gates the draft → active promotion of the same page', async () => { + const { protocol, rows } = makeProtocol(); + await expect(savePage(protocol, page(OVER), { mode: 'draft' })).resolves.toMatchObject({ success: true }); + + const err = await protocol.publishMetaItem({ type: 'page', name: 'deal_home' }).catch((e: any) => e); + + expect({ code: err?.code, status: err?.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect((err.issues ?? []).map((i: any) => i.rule)).toContain(RULE); + expect(activePageRows(rows)).toEqual([]); + }); + + it('(b) the door and `os build` give the SAME finding', async () => { + const { protocol } = makeProtocol(); + const err = await savePage(protocol, page(OVER)).catch((e: any) => e); + const atDoor = (err?.issues ?? []).filter((i: any) => i.rule === RULE); + + const stack = { pages: [page(OVER)] }; + const atBuild = runAuthoringRules('build', { normalized: stack, parsed: stack }) + .filter((f) => f.severity === 'error'); + + expect(atBuild.map((f) => f.rule)).toEqual([RULE]); + expect(atDoor).toHaveLength(1); + for (const k of ['rule', 'where', 'path', 'message', 'hint'] as const) { + expect(atDoor[0][k], `door and build disagree on '${k}'`).toBe(atBuild[0]![k]); + } + }); + + it('(c) CONTROL — a within-bounds tab predicate saves, and the row lands', async () => { + const { protocol, rows } = makeProtocol(); + + await expect(savePage(protocol, page(WITHIN))).resolves.toMatchObject({ success: true }); + + expect(activePageRows(rows)).toHaveLength(1); + const stack = { pages: [page(WITHIN)] }; + expect(runAuthoringRules('build', { normalized: stack, parsed: stack }).filter((f) => f.rule === RULE)).toEqual([]); + }); +}); + // A page component's `data` prop naming the retired view-level `api` provider, // at the runtime save door. `object-grid`, `object-map`, `object-gantt` and // `object-tree` take a view data source (`ViewDataSchema`) as `data`, so the