fix(cli, lint): gate a dangling lookup/master_detail reference, and resolve one across the artifact packages[]

.changeset/lookup-reference-target-gate.md

@@ -0,0 +1,25 @@
+---
+'@objectstack/lint': minor
+'@objectstack/cli': patch
+---
+
+`object-reference-unknown` now judges a field's `reference` — the target of `Field.lookup()` / `Field.masterDetail()` / `Field.user()` — with the same four-rung ladder it applies to every other object-name site, and `os build`'s per-package run resolves those names across the artifact's `packages[]`
+
+`FieldSchema.reference` is `z.string()`: the schema holds it present and non-empty on `lookup` / `master_detail`, and nothing anywhere asked whether the name resolved. So `os validate`, `os lint` and `os build` all exited 0 — no diagnostic of any severity — on `Field.lookup('zzz_object_that_does_not_exist')` (measured on 17.3.0), and the miss surfaced only at runtime: the record picker asking the REST layer for an object that is not registered (404 `OBJECT_NOT_FOUND`), `$expand` failing on the field, the form rendering a control that can never resolve a value.
+
+The site joins `validateObjectReferences` and rides its existing ladder, so the three commands judge it identically:
+
+1. resolves in the stack's own objects, or in the objects an entry of this artifact's `packages[]` provides → ok;
+2. resolves in `PLATFORM_PROVIDED_OBJECT_NAMES` (`sys_user`, the target `Field.user()` writes) → ok;
+3. unresolved and not platform-prefixed → **`error`** — `os validate` / `os build` / `os lint` exit 1;
+4. unresolved, platform-prefixed, registered by nothing (`sys_approval_process`) → the existing `object-reference-unregistered-platform` advisory.
+
+Judged: `lookup`, `master_detail`, `user`. Not judged, on purpose: `tree` (the object schema already refuses any target but the own name), a `reference` on a non-relationship type (inert), and `objectExtensions[].fields` (an extension targets an object another package owns, routinely one this artifact does not carry).
+
+## Migration
+
+**A build that used to pass can now fail.** Rung 3 is a new `error`-level refusal on a published accept set. Point the field at one of the stack's own objects, at an object another package of the same artifact ships, or at a platform object by its full name (`sys_user`, not `user`); the finding names the objects that resolve and suggests the nearest one.
+
+**A reference into a sibling package of the same release artifact resolves — it needs no annotation.** ADR-0130 makes the release artifact the co-ownership boundary, so `os build`'s per-package leg now hands each package's stack the artifact's `packages[]` as resolution context (`compile.ts`). A module's `crm_order.account` → its App package's `crm_account` is an ordinary rung-1 resolution on all three commands. This changes what a rule can resolve, never what it judges: the collections judged per package are still that package's own, and a name no entry of `packages[]` provides still errors on the per-package run exactly as it does on the union one.
+
+**A reference into another RELEASE ARTIFACT still has no rung** — an app naming an object a separate product ships (HotCLM's `clm_contract.crm_contract` → HotCRM). It is unresolved and unprefixed, so rung 3 refuses it. The declared escape for that case resolves against declared manifest dependencies and is its own change; ⛔ it is deliberately not an authored per-field marker, which would be a one-line switch that silences the gate.

packages/cli/src/commands/compile.ts

@@ -95,9 +95,40 @@ function artifactPackages(parsed: Record<string, unknown>): Array<{
  * would REFUSE, by name, every collection key this superset deliberately puts
  * under `manifest` — and re-opening it to stop the refusal would re-open the
  * real manifest surface with it.
+ *
+ * ## `packages` — the second key, and the only one (#16611)
+ *
+ * The body carries the collections this package OWNS. Judged with nothing else,
+ * a rule that resolves an object NAME concludes that a name a SIBLING package in
+ * the same artifact ships exists nowhere: `examples/app-multi-package`'s
+ * `crm_order.account` → `crm_account` — a lookup that fixture's README
+ * documents as the point of the fixture — is owned by its `core` package and
+ * read from its `orders` one. ADR-0130 makes the release artifact the
+ * co-ownership boundary, so a per-package pass that cannot see a sibling
+ * package's objects is THIS RUN's defect and not the author's; the ruled fix
+ * (director seat, decision batch #86) hands the per-package stack the
+ * artifact's own `packages[]` as RESOLUTION CONTEXT.
+ *
+ * Two properties make that safe, and they are the whole design:
+ *
+ *   1. ⛔ It changes what a rule can RESOLVE, never what it JUDGES. The
+ *      collections read off the top level are still this package's alone, so
+ *      every per-package finding this leg exists to produce is still produced.
+ *      ⛔ This is NOT "skip the site per package" — that would silence the gate
+ *      on the one command that ships.
+ *   2. A name no entry of `packages[]` provides resolves in neither run, so a
+ *      genuinely dangling reference still errors here exactly as it does in the
+ *      union run above.
+ *
+ * The array is passed through verbatim rather than reduced to a name list: the
+ * consuming rule owns which of an entry's contents are resolution context, and a
+ * set computed here would be a second copy of that decision, free to drift.
  */
-function packageBodyAsStack(body: Record<string, unknown>): Record<string, unknown> {
-  return { ...body, manifest: body };
+function packageBodyAsStack(
+  body: Record<string, unknown>,
+  artifactPackageEntries: unknown,
+): Record<string, unknown> {
+  return { ...body, manifest: body, packages: artifactPackageEntries };
 }
 
 /** Identity of one finding, for the per-package de-duplication below. */
@@ -437,6 +468,13 @@ export default class Compile extends Command {
       //     every finding twice and the author cannot tell a real per-package
       //     finding from an echo. What survives the filter is exactly the set
       //     the union could not see.
+      //
+      //     [#16611] Each package's stack is handed the artifact's `packages[]`
+      //     as RESOLUTION CONTEXT — see `packageBodyAsStack`. The list read here
+      //     is the one `artifactPackages` above walked, off the same parsed
+      //     stack, so the context a package resolves against is exactly the set
+      //     of packages this artifact will register (ADR-0130 D4/D5).
+      const artifactPackageEntries = (result.data as Record<string, unknown>).packages;
       const packageEntries = artifactPackages(result.data as Record<string, unknown>);
       if (packageEntries.length > 0) {
         if (!flags.json) {
@@ -445,7 +483,7 @@ export default class Compile extends Command {
         const alreadyReported = new Set(findings.map(findingKey));
         const perPackageErrors: Array<{ package: string } & typeof ruleErrors[number]> = [];
         for (const pkg of packageEntries) {
-          const asStack = packageBodyAsStack(pkg.body);
+          const asStack = packageBodyAsStack(pkg.body, artifactPackageEntries);
           const pkgFindings = runAuthoringRules('build', {
             normalized: asStack,
             parsed: asStack,

packages/lint/src/validate-object-references.test.ts

@@ -80,6 +80,231 @@ describe('validateObjectReferences — action params', () => {
   });
 });
 
+describe('validateObjectReferences — field relationship targets (#16611)', () => {
+  // The card's control probe, verbatim: on 17.3.0 `os validate`, `os lint` and
+  // `os build` all exited 0 on it, with no diagnostic of any severity.
+  it('errors on a lookup whose reference names an object declared nowhere', () => {
+    const stack = baseStack();
+    (stack.objects[0].fields as Record<string, unknown>).zzz_probe = {
+      type: 'lookup',
+      label: 'Control probe',
+      reference: 'zzz_object_that_does_not_exist',
+    };
+    const findings = validateObjectReferences(stack);
+    expect(findings).toHaveLength(1);
+    expect(findings[0].severity).toBe('error');
+    expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN);
+    expect(findings[0].where).toBe('object "crm_lead" · field "zzz_probe"');
+    expect(findings[0].path).toBe('objects[0].fields.zzz_probe.reference');
+    expect(findings[0].message).toContain('lookup target "zzz_object_that_does_not_exist"');
+    // The hint says what the miss costs at runtime, not just that it is a miss.
+    expect(findings[0].hint).toContain('record picker');
+  });
+
+  it('errors on a master_detail whose reference names an object declared nowhere', () => {
+    const stack = baseStack();
+    (stack.objects[0].fields as Record<string, unknown>).parent = {
+      type: 'master_detail',
+      required: true,
+      reference: 'crm_led',
+    };
+    const findings = validateObjectReferences(stack);
+    expect(findings).toHaveLength(1);
+    expect(findings[0].severity).toBe('error');
+    expect(findings[0].path).toBe('objects[0].fields.parent.reference');
+    expect(findings[0].message).toContain('master_detail target "crm_led"');
+    // One edit away from an own object: the suggester names it.
+    expect(findings[0].message).toContain('Did you mean "crm_lead"?');
+  });
+
+  it('accepts a lookup into an own object and the `Field.user()` shape (rungs ① and ③)', () => {
+    const stack = baseStack();
+    Object.assign(stack.objects[0].fields as Record<string, unknown>, {
+      account: { type: 'lookup', reference: 'crm_account' },
+      owner: { type: 'user', reference: 'sys_user' },
+      watchers: { type: 'lookup', reference: 'sys_user', multiple: true },
+    });
+    expect(validateObjectReferences(stack)).toEqual([]);
+  });
+
+  it('warns (not errors) on a platform-shaped target no package registers (rung ④)', () => {
+    const stack = baseStack();
+    (stack.objects[0].fields as Record<string, unknown>).approval = {
+      type: 'lookup',
+      reference: 'sys_approval_process',
+    };
+    const findings = validateObjectReferences(stack);
+    expect(findings).toHaveLength(1);
+    expect(findings[0].severity).toBe('warning');
+    expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNREGISTERED_PLATFORM);
+    expect(findings[0].path).toBe('objects[0].fields.approval.reference');
+    expect(findings[0].hint).toContain('sys_approval_request');
+  });
+
+  it('leaves the `reference` key alone on `tree` and on non-relationship types', () => {
+    // `tree`: the schema already refuses any target but the own name
+    // (`refuseForeignTreeReference`), so a survivor always resolves.
+    // `text`: the key is inert there; a finding would be about the wrong thing.
+    const stack = baseStack();
+    Object.assign(stack.objects[0].fields as Record<string, unknown>, {
+      parent: { type: 'tree', reference: 'crm_lead' },
+      note: { type: 'text', reference: 'zzz_object_that_does_not_exist' },
+    });
+    expect(validateObjectReferences(stack)).toEqual([]);
+  });
+
+  it('does not walk objectExtensions[].fields — cross-package by construction', () => {
+    // An extension adds fields to an object ANOTHER package owns; its targets
+    // are the cross-package case this ladder has no rung for, and the declared
+    // escape is its own card. Pinned so the boundary is a decision, not a gap.
+    const findings = validateObjectReferences({
+      ...baseStack(),
+      objectExtensions: [
+        {
+          object: 'crm_contract',
+          fields: { clause: { type: 'lookup', reference: 'zzz_object_that_does_not_exist' } },
+        },
+      ],
+    });
+    expect(findings).toEqual([]);
+  });
+
+  it('walks array-shaped and map-shaped field collections alike', () => {
+    const findings = validateObjectReferences({
+      objects: [
+        { name: 'crm_lead', fields: [{ name: 'owner', type: 'lookup', reference: 'user' }] },
+        { name: 'crm_account', fields: { rep: { type: 'lookup', reference: 'user' } } },
+      ],
+    });
+    expect(findings.map((f) => f.path)).toEqual([
+      'objects[0].fields.owner.reference',
+      'objects[1].fields.rep.reference',
+    ]);
+    for (const f of findings) {
+      expect(f.severity).toBe('error');
+      expect(f.hint).toContain('sys_user');
+    }
+  });
+
+  it('reports the field site before the sites that hang off the same object', () => {
+    const stack = baseStack();
+    (stack.objects[0].fields as Record<string, unknown>).zzz_probe = {
+      type: 'lookup',
+      reference: 'zzz_object_that_does_not_exist',
+    };
+    (stack.objects[0] as Record<string, unknown>).actions = [
+      { name: 'convert', params: [{ name: 'target', type: 'lookup', reference: 'accounts' }] },
+    ];
+    expect(validateObjectReferences(stack).map((f) => f.path)).toEqual([
+      'objects[0].fields.zzz_probe.reference',
+      'objects[0].actions[0].params[0].reference',
+    ]);
+  });
+});
+
+describe('validateObjectReferences — artifact packages[] as resolution context (#16611)', () => {
+  /**
+   * The shape `compile.ts`'s per-package leg hands the rules (ADR-0130 D4): ONE
+   * package's collections at the top level, and the whole artifact's
+   * `packages[]` beside them as context. Modelled on
+   * `examples/app-multi-package`, whose `orders` package reads `crm_account`
+   * out of its `core` sibling.
+   */
+  const ORDERS_BODY = {
+    id: 'com.example.multi.orders',
+    objects: [
+      {
+        name: 'crm_order',
+        fields: {
+          number: { type: 'text' },
+          account: { type: 'lookup', reference: 'crm_account' },
+        },
+      },
+    ],
+  };
+  const CORE_BODY = {
+    id: 'com.example.multi.core',
+    objects: [{ name: 'crm_account', fields: { name: { type: 'text' } } }],
+  };
+  const perPackageStack = (body: Record<string, unknown>, packages: unknown) => ({
+    ...body,
+    manifest: body,
+    packages,
+  });
+
+  it('CONTROL — the same package judged ALONE still errors, so the context is what does the work', () => {
+    // Without this leg "green" is indistinguishable from the rung having been
+    // switched off: this is the exact finding that reds `Build Core` on
+    // `examples/app-multi-package` when the ladder lands by itself.
+    const findings = validateObjectReferences(perPackageStack(ORDERS_BODY, undefined));
+    expect(findings).toHaveLength(1);
+    expect(findings[0].severity).toBe('error');
+    expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN);
+    expect(findings[0].path).toBe('objects[0].fields.account.reference');
+    expect(findings[0].message).toContain('lookup target "crm_account"');
+  });
+
+  it('resolves a lookup into an object a SIBLING package of the same artifact provides', () => {
+    const artifact = [{ manifest: ORDERS_BODY }, { manifest: CORE_BODY }];
+    expect(validateObjectReferences(perPackageStack(ORDERS_BODY, artifact))).toEqual([]);
+    // …and the sibling, judged from its own side, is unaffected.
+    expect(validateObjectReferences(perPackageStack(CORE_BODY, artifact))).toEqual([]);
+  });
+
+  it('NON-DEGENERACY — a name NO package in the artifact provides still errors', () => {
+    // The whole distinction between the ruled fix and "skip the field site per
+    // package": the site is still judged, against a wider set. `crm_contract`
+    // is the card's cross-REPO spelling — no entry here ships it.
+    const dangling = {
+      ...ORDERS_BODY,
+      objects: [
+        { name: 'crm_order', fields: { contract: { type: 'lookup', reference: 'crm_contract' } } },
+      ],
+    };
+    const findings = validateObjectReferences(
+      perPackageStack(dangling, [{ manifest: dangling }, { manifest: CORE_BODY }]),
+    );
+    expect(findings).toHaveLength(1);
+    expect(findings[0].severity).toBe('error');
+    expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN);
+    expect(findings[0].path).toBe('objects[0].fields.contract.reference');
+    // The remedy lists what the ARTIFACT provides, not only this package's own
+    // objects — that is the list the author actually has to choose from.
+    expect(findings[0].hint).toContain('Defined objects: crm_account, crm_order.');
+  });
+
+  it('carries the context to every site on the rule, not only the field one', () => {
+    const withNav = {
+      ...ORDERS_BODY,
+      navigation: [{ name: 'accounts', objectName: 'crm_account', requiresObject: 'crm_account' }],
+      actions: [{ name: 'link', params: [{ name: 'a', type: 'lookup', reference: 'crm_account' }] }],
+    };
+    expect(
+      validateObjectReferences(
+        perPackageStack(withNav, [{ manifest: withNav }, { manifest: CORE_BODY }]),
+      ),
+    ).toEqual([]);
+  });
+
+  it('reads only names a package REALLY declares — an entry with no body contributes none', () => {
+    // A future `{ ref, integrity }` external segment carries no manifest
+    // content (ADR-0130 D4). Inventing a name for such an entry would silence
+    // the ladder, which is the one mistake this context must not make.
+    const findings = validateObjectReferences(
+      perPackageStack(ORDERS_BODY, [{ ref: 'sha256-x' }, { manifest: { id: 'x' } }, 'not-an-entry']),
+    );
+    expect(findings).toHaveLength(1);
+    expect(findings[0].path).toBe('objects[0].fields.account.reference');
+  });
+
+  it('ignores a `packages` value that is not a list of entries', () => {
+    for (const packages of [null, 42, 'core']) {
+      const findings = validateObjectReferences(perPackageStack(ORDERS_BODY, packages));
+      expect(findings.map((f) => f.path)).toEqual(['objects[0].fields.account.reference']);
+    }
+  });
+});
+
 describe('validateObjectReferences — dashboard global filters', () => {
   // The other HotCRM `object: 'user'` instance.
   it('errors on optionsFrom.object naming a nonexistent object', () => {