fix(lint)!: os build and the object_extension publish door refuse a formula cycle an object extension adds

.changeset/33153-extension-formula-cycle.md

@@ -0,0 +1,22 @@
+---
+'@objectstack/lint': minor
+'@objectstack/spec': minor
+---
+
+fix(lint): `os build` and the runtime `object_extension` publish door refuse a formula an object extension adds that reads its own value, itself or through another formula, as they refuse one the object declares
+
+Clause-②: no (narrowing) — `os build`, `os validate`, `os lint` and the `object_extension` publish door (Studio, `PUT /api/v1/meta/object_extension/TARGET`, the MCP and AI authoring doors) now refuse one shape an extension could carry before: a `formula` field an extension adds that sits on a cycle of formulas (itself, two added formulas reading each other, or a cycle closing through a formula the target declares). No exported type or signature changes. Measured first-party: 0 `objectExtensions` entries add a `formula` field (`examples/app-showcase` one entry, a `select`, a `url` and a `number` field; `packages/plugins/plugin-auth` one, a `secret` field; HotCRM none), so nothing first-party is newly refused.
+Metadata-protocol: narrowed
+Skills-docs: none owed — this PR edits `content/docs/deployment/validating-metadata.mdx` (the object extension paragraph names the cycle check); `skills/objectstack-formula` and `skills/objectstack-data` teach formula fields and `defineObjectExtension({ extend, fields })` without saying an extension is exempt from the cycle check, so the build and the door now give the verdict the object's own formulas already get.
+
+<!-- adr-0087: registered object-extension-formula-cycle-refused -->
+
+**BREAKING** accept-set narrowing of `os build`, `os validate`, `os lint` and the runtime `object_extension` publish door.
+
+**Before:** the formula-cycle check ran on the object walk and named the object's declared formulas only. A formula an extension adds that reads itself (`a__c = record.a__c`), or two that read each other, was accepted at the build and the door; the engine never evaluated them, and they read null on every row.
+
+**Now:** the cycle is found on the target as served (its declared fields with every kept extension's fields folded in), and each member an extension adds is refused with the object's own verdict and sentence (at the door, a name the target already serves included: the written definition is the one served) (`expression-invalid`), located at the entry: `object 'TARGET' · objectExtensions[0] · field 'FIELD' expression` (`packages[j].manifest.` prefixed in a built artifact). It runs at the `object_extension` door (`422 INVALID_METADATA`) as in `os build`.
+
+**Unchanged:** a cycle among the object's own formulas, at the same location; a member the target declares on a cycle an extension closes keeps its location at the object, named once; a draft save (never gated); a stored row (judged only when it is published again). At the extension door a cycle among the target's own formulas, or a sibling package's, is not charged to the extension write, and a formula that only reads a cycle is not refused. In `os build` on an object the stack declares, a field two packages' extensions add at the same priority is not judged, since which definition the registry serves is registration order, so a cycle through it is not found there; on an object the stack does not declare, the later entry's definition is judged. The publish door judges the written entry's own definition of every field it adds, whatever priority another package's extension adds the same field at, so it can refuse a tie, or an entry another package outranks, that `os build` lets through.
+
+**Fix:** before building or publishing again, break the cycle: have one formula on it read stored fields, or formulas that do not read it back.

content/docs/deployment/validating-metadata.mdx

@@ -618,7 +618,16 @@ read through a lookup the entry adds (`record.manager__c.LEAF`) is judged as
 one through a lookup the target declares (#33216), and a `format` rule's regex
 and a `json_schema` rule's schema and format names are checked as on the
 object's own rules (#33217), with the same rule ids, at the door and in
-`os build` alike.
+`os build` alike. A formula the entry adds that reads its own value, itself or
+through another formula, is refused as one the object declares is (#33153):
+the cycle is found on the target as served, each member the entry adds is
+located at the entry (`object 'TARGET' · objectExtensions[0] · field 'FIELD'
+expression`), and in `os build` a member the target declares keeps its
+location at the object. The door judges the written entry's own definition of
+every field it adds, so a name the target already serves is refused at the
+entry too, whatever priority another package's extension adds it at; `os build`
+judges the definition the registry serves, and on a declared object leaves out
+a field two packages add at the same priority.
 
 **Declared enforcement that cannot run** is two rows for the mirror-image
 reason, and #4716 is where it split. That heading covers four rule ids, and

packages/lint/src/formula-cycle-issues.ts

@@ -51,12 +51,14 @@ function formulaSourceOf(def: AnyRec): string | undefined {
  * (`object 'x_job' · field 'f_total' expression`). `fields` are the object's
  * fields by name, as served (an `objectExtensions` field included, so a cycle
  * through one is seen); only the names in `report` get a finding, so each is
- * located on a field the caller's walk names.
+ * located on a field the caller's walk names. [#33153] `report` given as a map
+ * names each reported field's location itself: the extension walk's, from
+ * {@link extensionFormulaLoci}.
  */
 export function formulaCycleIssues(
   objectName: string,
   fields: ReadonlyMap<string, AnyRec>,
-  report: ReadonlySet<string>,
+  report: ReadonlySet<string> | ReadonlyMap<string, string>,
 ): FormulaCycleIssue[] {
   const sources = new Map<string, string>();
   for (const [name, def] of fields) {
@@ -78,7 +80,39 @@ export function formulaCycleIssues(
   for (const [field, source] of sources) {
     const message = cycles.get(field);
     if (message === undefined || !report.has(field)) continue;
-    issues.push({ where: `object '${objectName}' · field '${field}' expression`, message, source, severity: 'error' });
+    const where = report instanceof Map ? report.get(field) : undefined;
+    issues.push({ where: where ?? `object '${objectName}' · field '${field}' expression`, message, source, severity: 'error' });
   }
   return issues;
 }
+
+/**
+ * [#33153] Where the extension walk locates a cycle member an
+ * `objectExtensions` entry adds to `target`: each field name an entry adds,
+ * mapped to that entry's location for it
+ * (`object 'T' · objectExtensions[0] · field 'f' expression`, the location
+ * the field-formula pass gives the same field). `entries` are the target's
+ * entries in fold order (`objectExtensionsByTarget`), so a name two entries
+ * add is located at the later one, the definition the registry serves. A
+ * name in `left` is left out: the caller's names whose member is located
+ * elsewhere. The build passes the names the authored target declares (the
+ * object walk locates those at the object, so a cycle closing through a
+ * declared formula keeps that location); the `object_extension` door passes
+ * none, since it judges the written entry's own definition of every name the
+ * entry adds, over everything the target served before the write.
+ */
+export function extensionFormulaLoci(
+  target: string,
+  entries: ReadonlyArray<{ path: string; fields: readonly AnyRec[] }>,
+  left: ReadonlySet<string>,
+): Map<string, string> {
+  const loci = new Map<string, string>();
+  for (const entry of entries) {
+    for (const field of entry.fields) {
+      const name = field.name;
+      if (typeof name !== 'string' || left.has(name)) continue;
+      loci.set(name, `object '${target}' · ${entry.path} · field '${name}' expression`);
+    }
+  }
+  return loci;
+}

packages/lint/src/runtime-gate.object-formula-cycle.test.ts

@@ -93,3 +93,159 @@ describe('#33100 — formula cycles at the object door and in the build', () =>
     expect({ where: built[0]!.where, message: built[0]!.message }).toEqual({ where: door[0]!.where, message: door[0]!.message });
   });
 });
+
+describe('#33153 — formula cycles an object extension adds, at the object_extension door and in the build', () => {
+  /** The extended object, as the door's live universe carries it, with the formulas given. */
+  const target = (formulas: Record<string, unknown> = {}) => xJob(formulas);
+  const extension = (fields: Record<string, unknown>) => ({ extend: 'x_job', fields });
+  const extWhere = (field: string, at = 0) => `object 'x_job' · objectExtensions[${at}] · field '${field}' expression`;
+  const gateExtension = (item: unknown, objects: unknown[] = [target()]) =>
+    runRuntimeAuthoringRules({ type: 'object_extension', item, context: { objects } });
+  const build = (stack: Record<string, unknown>) =>
+    cycleFindings(runAuthoringRules('build', { normalized: stack, parsed: stack }));
+  const located = (fs: ReadonlyArray<{ where: string; message: string }>) => fs.map(({ where: w, message }) => ({ where: w, message }));
+
+  it('⭐ LIT — an extension formula reading itself is REFUSED at the door and the build, located at the entry, in the engine\'s words', () => {
+    const item = extension({ a__c: formula('record.a__c') });
+
+    const door = cycleFindings(gateExtension(item).errors);
+    expect(door, dump(door)).toHaveLength(1);
+    expect(door[0]).toMatchObject({ severity: 'error', where: extWhere('a__c') });
+    expect(door[0]!.message).toContain(formulaCycleMessage('a__c', ['a__c']));
+
+    const built = build({ objects: [target()], objectExtensions: [item] });
+    expect(located(built)).toEqual(located(door));
+  });
+
+  it('⭐ LIT — two extension formulas reading each other are REFUSED on each, at the door and the build', () => {
+    const item = extension({ a__c: formula('record.b__c'), b__c: formula('record.a__c') });
+
+    const door = cycleFindings(gateExtension(item).errors);
+    expect(door.map((f) => f.where), dump(door)).toEqual([extWhere('a__c'), extWhere('b__c')]);
+    expect(door[0]!.message).toContain(formulaCycleMessage('a__c', ['a__c', 'b__c']));
+    expect(door[1]!.message).toContain(formulaCycleMessage('b__c', ['a__c', 'b__c']));
+
+    expect(located(build({ objects: [target()], objectExtensions: [item] }))).toEqual(located(door));
+  });
+
+  it('⭐ LIT — a cycle closing through a declared formula: the declared member keeps its location at the object, the added one is located at the entry', () => {
+    const declared = target({ f_total: formula('record.e__c * 2') });
+    const item = extension({ e__c: formula('record.f_total + 1') });
+
+    // The door judges the extension write: the member it adds.
+    const door = cycleFindings(gateExtension(item, [declared]).errors);
+    expect(door.map((f) => f.where), dump(door)).toEqual([extWhere('e__c')]);
+    expect(door[0]!.message).toContain(formulaCycleMessage('e__c', ['e__c', 'f_total']));
+
+    // The build names each member once, where it is authored.
+    const built = build({ objects: [declared], objectExtensions: [item] });
+    expect(built.map((f) => f.where), dump(built)).toEqual([where('f_total'), extWhere('e__c')]);
+  });
+
+  it('a cycle across two packages\' extensions is named at each entry, by the build', () => {
+    const stack = {
+      objects: [target()],
+      packages: [
+        { manifest: { objectExtensions: [extension({ a__c: formula('record.b__c') })] } },
+        { manifest: { objectExtensions: [extension({ b__c: formula('record.a__c') })] } },
+      ],
+    };
+    const built = build(stack);
+    expect(built.map((f) => f.where), dump(built)).toEqual([
+      "object 'x_job' · packages[0].manifest.objectExtensions[0] · field 'a__c' expression",
+      "object 'x_job' · packages[1].manifest.objectExtensions[0] · field 'b__c' expression",
+    ]);
+  });
+
+  it('⭐ LIT — the door refuses a member under a name the target already serves: a re-declared own field', () => {
+    // A package extending an object it owns and re-declaring one of its
+    // declared fields as a formula that reads itself: the written definition
+    // is the one served, so the door refuses it at the entry.
+    const declared = target();
+    const item = extension({ quantity: formula('record.quantity + 1') });
+
+    const door = cycleFindings(gateExtension(item, [declared]).errors);
+    expect(door.map((f) => f.where), dump(door)).toEqual([extWhere('quantity')]);
+    expect(door[0]!.message).toContain(formulaCycleMessage('quantity', ['quantity']));
+
+    // The build refuses the same stack once, at the object, where the object
+    // walk locates the declared name.
+    const built = build({ objects: [declared], objectExtensions: [item] });
+    expect(built.map((f) => f.where), dump(built)).toEqual([where('quantity')]);
+  });
+
+  it('⭐ LIT — the door refuses a member under a name a sibling package\'s published extension already serves', () => {
+    // The door's live target carries the sibling's `s__c` (a stored number);
+    // the written entry adds `s__c` over it as a formula reading itself.
+    const served = target({ s__c: { type: 'number', label: 'Sibling' } });
+    const item = { ...extension({ s__c: formula('record.s__c * 2') }), priority: 300 };
+
+    const door = cycleFindings(gateExtension(item, [served]).errors);
+    expect(door.map((f) => f.where), dump(door)).toEqual([extWhere('s__c')]);
+
+    // The build over both packages serves the later writer and refuses it at its entry.
+    const built = build({
+      objects: [target()],
+      packages: [
+        { manifest: { objectExtensions: [{ ...extension({ s__c: { type: 'number', label: 'Sibling' } }), priority: 100 }] } },
+        { manifest: { objectExtensions: [item] } },
+      ],
+    });
+    expect(built.map((f) => f.where), dump(built)).toEqual([
+      "object 'x_job' · packages[1].manifest.objectExtensions[0] · field 's__c' expression",
+    ]);
+  });
+
+  it('the door judges the written entry\'s own definition whatever a sibling\'s priority: a tie and an outranked entry are refused there, though the build lets both through (#33494)', () => {
+    // The door's live target carries the sibling's `s__c` as a plain field,
+    // with no priority, and folds the written entry over it.
+    const served = target({ s__c: { type: 'number', label: 'Sibling' } });
+    const sibling = (priority: number) => ({ ...extension({ s__c: { type: 'number', label: 'Sibling' } }), priority });
+    const written = (priority: number) => ({ ...extension({ s__c: formula('record.s__c * 2') }), priority });
+    const twoPackages = (siblingPriority: number, writtenPriority: number) => ({
+      objects: [target()],
+      packages: [
+        { manifest: { objectExtensions: [sibling(siblingPriority)] } },
+        { manifest: { objectExtensions: [written(writtenPriority)] } },
+      ],
+    });
+
+    for (const [label, siblingPriority, writtenPriority] of [['a tie', 200, 200], ['outranked', 300, 200]] as const) {
+      const door = cycleFindings(gateExtension(written(writtenPriority), [served]).errors);
+      expect(door.map((f) => f.where), `${label}: ${dump(door)}`).toEqual([extWhere('s__c')]);
+      // The build drops the tied name on a declared target, and serves the
+      // outranking sibling's number: neither leaves a cycle to name.
+      const built = build(twoPackages(siblingPriority, writtenPriority));
+      expect(built, `${label}: ${dump(built)}`).toEqual([]);
+    }
+  });
+
+  it('a sibling package\'s stored cycle, and a formula that only reads a cycle, are not the extension write\'s to answer for', () => {
+    const served = target({ s_x: formula('record.s_y + 1'), s_y: formula('record.s_x + 1') });
+    const result = gateExtension(extension({ e__c: formula('record.s_x * 2') }), [served]);
+    expect(result.errors.filter((f) => f.rule === EXPRESSION_INVALID), dump(result)).toEqual([]);
+  });
+
+  it('an extension of an object the door does not carry is judged on its own fields', () => {
+    const door = cycleFindings(gateExtension(extension({ a__c: formula('record.a__c') }), []).errors);
+    expect(door.map((f) => f.where), dump(door)).toEqual([extWhere('a__c')]);
+  });
+
+  it('the target\'s own stored cycle is not the extension write\'s to answer for (the differential)', () => {
+    const cyclic = target({ f_x: formula('record.f_y + 1'), f_y: formula('record.f_x + 1') });
+    const result = gateExtension(extension({ e__c: formula('record.quantity * 2') }), [cyclic]);
+    expect(result.errors, dump(result)).toEqual([]);
+  });
+
+  it('⭐ CONTROL — an extension formula over a declared formula, and a chain inside the extension, publish clean', () => {
+    const declared = target({ f_total: formula('record.quantity * record.unit_price') });
+    for (const item of [
+      extension({ e__c: formula('record.f_total + 1') }),
+      extension({ b__c: formula('record.a__c * 2'), a__c: formula('record.quantity + 1') }),
+    ]) {
+      const result = gateExtension(item, [declared]);
+      expect(result.errors.filter((f) => f.rule === EXPRESSION_INVALID), dump(result)).toEqual([]);
+      expect(build({ objects: [declared], objectExtensions: [item] })).toEqual([]);
+    }
+  });
+});

packages/lint/src/validate-expressions.ts

@@ -161,7 +161,7 @@ import type { NullGuardOptions, NullGuardOutcome } from './validate-null-guards.
 import { stripInjectedSystemColumns } from '@objectstack/metadata-core';
 import { objectExtensionsByTarget, objectsAsServed, objectsWithoutTiedFields, recordsOf } from './object-graph.js';
 import { storedExpressionInputIssues } from './stored-expression-inputs.js';
-import { formulaCycleIssues } from './formula-cycle-issues.js';
+import { extensionFormulaLoci, formulaCycleIssues } from './formula-cycle-issues.js';
 export interface ExprIssue {
   where: string;
   message: string;
@@ -4174,6 +4174,33 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
         });
       }
     }
+    // [#33153] The formula-cycle pass, over the target as served
+    // (`fieldDefs`: on a declared target its served body, every kept entry's
+    // fields folded in and a field two packages tie on left out; on an
+    // undeclared one the entries' own fields, the later entry's definition of
+    // a shared name), so a formula an entry adds that reads itself, or two that
+    // read each other, are refused as the object's own are (#33100). Each
+    // member an entry adds is located at that entry
+    // ({@link extensionFormulaLoci}).
+    //
+    // In the build `base` is the authored object, and a member it declares is
+    // the object walk's, located at the object: a cycle closing through a
+    // declared formula keeps that location and is named once. On the door's
+    // one pass `base` is the target as served BEFORE the write (other
+    // packages' published extension fields included, carried as plain fields
+    // with no priority) and the object walk does not run, while the written
+    // entry, its sole extension, is folded over it. So the door judges the
+    // written entry's own definition of every name it adds, a name the target
+    // already serves included, and none is left out there. Where a sibling
+    // package adds the same name at the same priority (a tie, which the build
+    // drops on a declared target) or at a higher one (the build serves the
+    // sibling's), the door can refuse a cycle the build lets through; making
+    // the door read the build's winner is #33494. A member the target or a
+    // sibling package stored is never a name the entry adds, so the target's
+    // own cycle, a sibling's, and a formula that only reads a cycle are not
+    // this write's to answer for.
+    const ownNames = new Set(base && !extensionWrite ? fieldEntries(base).map(([n]) => n) : []);
+    issues.push(...formulaCycleIssues(target, fieldDefs, extensionFormulaLoci(target, entries, ownNames)));
   }
 
   // [#33072] An extension write's one pass ends here.