fix(reference-carrier): four C2 readers answer absence instead of propagating an unreadable reference

.changeset/19081-reference-carrier-c2-readers.md

@@ -0,0 +1,21 @@
+---
+"@objectstack/plugin-approvals": patch
+"@objectstack/service-analytics": patch
+"@objectstack/cli": patch
+---
+
+Four readers of `FieldSchema.reference` gated the carrier with a truthiness test and then **propagated** it. `FieldSchema.reference` is declared an optional **string**, so the answer a reader owes for a carrier it cannot read is absence — and one of these four did worse than lose the information, it invented a name for it:
+
+```
+out.push({ key, reference: String(f.reference) })   // -> reference: '[object Object]'
+```
+
+Each site now reads the carrier through the one arbiter, `referenceCarrierOf`, and catches its refusal **at the site** — so the reader answers absence and reports, instead of aborting. That is the deliberate difference from `@objectstack/objectql`'s cascade seams, which let the same refusal propagate: those assert something positive about the schema on a write path, while these four are best-effort display and diagnostic readers whose own failure handling would have turned one unreadable field into a much wider loss.
+
+- **`@objectstack/plugin-approvals`** — `resolveLookupFields`. The stringified carrier was handed on as an object name to `engine.find()`, where it could never resolve and the failure was swallowed by the caller's `catch`. The field is now left out of the inbox display enrichment and logged; readable targets are unaffected. It is dropped rather than carried with an absent target because the sole consumer uses `reference` as the object name and has nothing to do with an entry carrying none.
+- **`@objectstack/service-analytics`** — the ADR-0021 relationship → target-object resolver. An unreadable carrier became the joined table for a dataset's `include`; the resolver now answers `undefined`, which its existing fallback turns into the compiler's own refusal, plus one warning naming the field.
+- **`@objectstack/cli`** — `os doctor`'s circular-dependency and unused-object checks, which put the carrier into a graph node and a name set. Both now report the unreadable carrier as a finding rather than skipping it, because "no circular references detected" and "defined but not referenced" are positive claims that an edge nobody could read cannot support. The same file's `collectViewObjectRefs` already narrowed its carrier this way.
+
+`null`, `undefined` and `''` are absence, not a wrong shape, and still pass silently at every one of these sites — a field is allowed to name no target. Each site's absence answer and its readable-target answer are pinned alongside the refusal.
+
+Upgrading: nothing conformant changes. A non-string `reference` is refused by `ObjectSchema.safeParse`, so a value in that shape only ever reaches these readers without having passed parse at all.

packages/cli/src/commands/doctor.ts

@@ -7,6 +7,7 @@ import dotenvFlow from 'dotenv-flow';
 import fs from 'fs';
 import path from 'path';
 import { normalizeStackInput } from '@objectstack/spec';
+import { referenceCarrierOf } from '@objectstack/spec/data';
 import { printHeader, printSuccess, printWarning, printError, printStep, printInfo } from '../utils/format.js';
 import { loadConfig, configExists } from '../utils/config.js';
 import { checkProtocolVersionGap } from '../utils/protocol-version-gap.js';
@@ -700,17 +701,37 @@ export function resolveTenancyPostureOrFinding(reading: DotenvReading): TenancyP
 
 // ─── Config-Aware Checks ────────────────────────────────────────────
 
-function detectCircularDependencies(objects: any[]): string[] {
+// Exported for the pin on its carrier reading below; `doctor` itself is the
+// only caller.
+export function detectCircularDependencies(objects: any[]): string[] {
   const issues: string[] = [];
   const graph = new Map<string, string[]>();
 
   for (const obj of objects) {
     const deps: string[] = [];
     if (obj.fields && typeof obj.fields === 'object') {
-      for (const field of Object.values(obj.fields) as any[]) {
-        if (field?.type === 'lookup' && field?.reference) {
-          deps.push(field.reference);
+      for (const [key, field] of Object.entries(obj.fields) as Array<[string, any]>) {
+        if (field?.type !== 'lookup') continue;
+        // The carrier is read through the ONE arbiter, the same narrowing
+        // `collectViewObjectRefs` below already performs — a truthiness gate
+        // admitted an object- or array-valued `reference` as a NODE of the
+        // dependency graph, where it can never match an object name and prints
+        // as `[object Object]` in a cycle message. Absence is the contract's
+        // answer; unreadability is reported, because this check's success line
+        // ("No circular references detected") asserts something positive that a
+        // silently missing edge cannot support. The throw is caught so `doctor`
+        // keeps reporting on exactly the broken metadata it exists to inspect.
+        let reference: string | undefined;
+        try {
+          reference = referenceCarrierOf(field, 'doctor.detectCircularDependencies');
+        } catch (err: any) {
+          issues.push(
+            `Object "${obj.name}" field "${key}": lookup target is unreadable, so this edge is absent `
+            + `from the dependency graph — ${err?.message ?? err}`,
+          );
+          continue;
         }
+        if (reference) deps.push(reference);
       }
     }
     graph.set(obj.name, deps);
@@ -890,13 +911,31 @@ export function findUnusedObjects(config: any): string[] {
   }
 
   // Lookup fields reference other objects
+  //
+  // The carrier is read through the ONE arbiter rather than a truthiness gate:
+  // an unreadable `reference` used to enter `referencedObjects` as a non-string
+  // member, where it marks nothing as referenced and so lets this function
+  // report the object it actually points at as unused. Unreadability is
+  // REPORTED rather than skipped, because "defined but not referenced" is a
+  // positive claim about the config and an edge nobody could read cannot
+  // support it. The throw is caught so `doctor` keeps reporting.
+  const unreadableCarriers: string[] = [];
   if (Array.isArray(config.objects)) {
     for (const obj of config.objects) {
       if (obj.fields && typeof obj.fields === 'object') {
-        for (const field of Object.values(obj.fields) as any[]) {
-          if (field?.type === 'lookup' && field?.reference) {
-            referencedObjects.add(field.reference);
+        for (const [key, field] of Object.entries(obj.fields) as Array<[string, any]>) {
+          if (field?.type !== 'lookup') continue;
+          let reference: string | undefined;
+          try {
+            reference = referenceCarrierOf(field, 'doctor.findUnusedObjects');
+          } catch (err: any) {
+            unreadableCarriers.push(
+              `Object "${obj.name}" field "${key}": lookup target is unreadable, so it marks no object `
+              + `as referenced — ${err?.message ?? err}`,
+            );
+            continue;
           }
+          if (reference) referencedObjects.add(reference);
         }
       }
     }
@@ -908,7 +947,7 @@ export function findUnusedObjects(config: any): string[] {
       unused.push(`Object "${name}" is defined but not referenced by any view, flow, app, or lookup field`);
     }
   }
-  return unused;
+  return [...unreadableCarriers, ...unused];
 }
 
 // ─── ADR-0120 D5e — `isolated`-posture unique-scope advisory ────────

packages/cli/test/doctor-reference-carrier.test.ts

@@ -0,0 +1,75 @@
+// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license.
+//
+// Two of `os doctor`'s config checks read a lookup field's target through a
+// truthiness gate — `if (field?.type === 'lookup' && field?.reference)` — and
+// then put the value straight into a graph node (`detectCircularDependencies`)
+// or a name set (`findUnusedObjects`). An object- or array-valued `reference`
+// passes truthiness, so a non-string entered both, where it matches no object
+// name and renders as `[object Object]` in a cycle message.
+//
+// `collectViewObjectRefs`, twenty lines away in the same file, already narrowed
+// its carrier with `typeof … === 'string'`; these two now read it through the
+// same arbiter the rest of the platform uses. Unreadability is REPORTED rather
+// than skipped, because both checks publish a positive verdict — "no circular
+// references detected", "defined but not referenced" — that an edge nobody
+// could read cannot support.
+
+import { describe, it, expect } from 'vitest';
+import { detectCircularDependencies, findUnusedObjects } from '../src/commands/doctor';
+
+/** An `ObjectSchema` literal where the target NAME belongs. */
+const UNREADABLE = { name: 'crm_account', fields: {} };
+
+const obj = (name: string, fields: Record<string, unknown>) => ({ name, label: name, fields });
+
+describe('doctor.detectCircularDependencies — an unreadable `reference` carrier', () => {
+  it('never puts a non-string into the dependency graph, and says so', () => {
+    const issues = detectCircularDependencies([
+      obj('crm_contact', { account: { type: 'lookup', reference: UNREADABLE } }),
+      obj('crm_account', {}),
+    ]);
+    expect(issues.join('\n')).not.toContain('[object Object]');
+    expect(issues).toHaveLength(1);
+    expect(issues[0]).toContain('crm_contact');
+    expect(issues[0]).toContain('account');
+    expect(issues[0]).toContain('doctor.detectCircularDependencies');
+  });
+
+  it('still detects a cycle built from readable targets', () => {
+    const issues = detectCircularDependencies([
+      obj('crm_contact', { account: { type: 'lookup', reference: 'crm_account' } }),
+      obj('crm_account', { primary_contact: { type: 'lookup', reference: 'crm_contact' } }),
+    ]);
+    expect(issues).toHaveLength(1);
+    expect(issues[0]).toContain('Circular dependency');
+  });
+
+  it('stays silent for a lookup that legitimately names no target', () => {
+    expect(detectCircularDependencies([obj('crm_contact', { account: { type: 'lookup' } })])).toEqual([]);
+  });
+});
+
+describe('doctor.findUnusedObjects — an unreadable `reference` carrier', () => {
+  const config = (reference: unknown) => ({
+    objects: [
+      obj('crm_contact', { account: { type: 'lookup', reference } }),
+      obj('crm_account', { title: { type: 'text' } }),
+    ],
+    views: [{ list: { type: 'grid', data: { provider: 'object', object: 'crm_contact' } } }],
+  });
+
+  it('reports the unreadable carrier rather than letting it distort the verdict', () => {
+    const found = findUnusedObjects(config(UNREADABLE));
+    expect(found.join('\n')).not.toContain('[object Object]');
+    // The carrier finding is present, and names the field that carries it.
+    expect(found.some(m => m.includes('crm_contact') && m.includes('account') && m.includes('unreadable')))
+      .toBe(true);
+    expect(found.some(m => m.includes('doctor.findUnusedObjects'))).toBe(true);
+  });
+
+  it('still counts a readable lookup target as a reference', () => {
+    // `crm_account` is referenced ONLY by the lookup, so this is the control
+    // that separates "narrowed" from "this path was closed".
+    expect(findUnusedObjects(config('crm_account'))).toEqual([]);
+  });
+});

packages/plugins/plugin-approvals/src/approval-service.ts

@@ -56,7 +56,7 @@ import type {
 // fields the caller had already supplied.
 import type { ExecutionContext } from '@objectstack/spec/kernel';
 import { RESUME_AUTHORITY_SERVICE } from '@objectstack/spec/contracts';
-import { isFileIdToken } from '@objectstack/spec/data';
+import { isFileIdToken, referenceCarrierOf } from '@objectstack/spec/data';
 // [#11993] The SANCTIONED renderer for OPERATION-level refusal copy. The
 // Operation Message Catalog is the ONE seat for these sentences — its own
 // header bars both a package-local string table and a second rendering
@@ -5769,9 +5769,37 @@ export class ApprovalService implements IApprovalService {
       const fields = schema?.fields ?? {};
       const out: Array<{ key: string; reference: string }> = [];
       for (const [key, f] of Object.entries<any>(fields)) {
-        if ((f?.type === 'lookup' || f?.type === 'master_detail' || f?.type === 'user') && f?.reference) {
-          out.push({ key, reference: String(f.reference) });
+        if (f?.type !== 'lookup' && f?.type !== 'master_detail' && f?.type !== 'user') continue;
+        // The carrier is read through the ONE arbiter instead of a truthiness
+        // gate. `String()` on an object-valued `reference` produced the literal
+        // target name `'[object Object]'`, and the sole consumer below hands
+        // `reference` straight to `engine.find(<object name>)` — so an
+        // unreadable carrier became a query for an object that can never exist,
+        // swallowed by that consumer's own `catch`. Absence is the contract's
+        // answer (`FieldSchema.reference` is an optional STRING) and is what
+        // this now yields.
+        //
+        // The throw is caught PER FIELD, which is the deliberate difference
+        // between this reader and the cascade seams in `@objectstack/objectql`
+        // that let `referenceCarrierOf` propagate: those assert something
+        // positive about the schema on a write path, while this is a
+        // best-effort display enrichment whose outer `catch` returns `[]` —
+        // letting the throw reach it would drop EVERY lookup field of the
+        // object over one unreadable carrier. The entry is dropped rather than
+        // pushed with `reference` absent because the consumer uses `reference`
+        // as the object name argument and has nothing to do with an entry that
+        // carries none.
+        let reference: string | undefined;
+        try {
+          reference = referenceCarrierOf(f, 'ApprovalService.resolveLookupFields');
+        } catch (err: any) {
+          this.logger?.warn?.(
+            `[approvals] lookup field "${object}.${key}" left out of inbox display enrichment: `
+            + `${err?.message ?? err}`,
+          );
+          continue;
         }
+        if (reference) out.push({ key, reference });
       }
       return out;
     } catch { return []; }

packages/plugins/plugin-approvals/src/lookup-field-reference-carrier.test.ts

@@ -0,0 +1,86 @@
+// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license.
+//
+// `resolveLookupFields` used to gate the carrier with a truthiness test and
+// then stringify it: `out.push({ key, reference: String(f.reference) })`. An
+// object-valued `reference` passes truthiness and `String()` renders it as the
+// literal target name `[object Object]` — a name that can never resolve, handed
+// on to `engine.find(<object name>)` by the inbox display enrichment and lost
+// inside that caller's own `catch`.
+//
+// `FieldSchema.reference` is declared an optional STRING, so the answer a
+// reader owes for an unreadable carrier is ABSENCE. These cases pin both
+// halves: the unreadable field is left out and reported, and a readable one is
+// still carried through unchanged — otherwise "fixed" and "this path is now
+// closed" would be indistinguishable.
+
+import { describe, it, expect, vi } from 'vitest';
+import { ApprovalService } from './approval-service.js';
+
+/** The one engine member `resolveLookupFields` reads. */
+const engineWithSchema = (fields: Record<string, unknown>) => ({
+  getSchema: (_object: string) => ({ fields }),
+}) as any;
+
+const makeService = () => {
+  const warn = vi.fn();
+  const service = new ApprovalService({
+    engine: engineWithSchema({
+      // Readable: the control that keeps this a narrowing rather than a shutdown.
+      account: { type: 'lookup', reference: 'crm_account' },
+      // Unreadable: an `ObjectSchema` literal where the target NAME belongs.
+      // `ObjectSchema.safeParse` refuses this at the contract door, so a value
+      // in this shape reached the reader without ever passing parse.
+      broken: { type: 'lookup', reference: { name: 'shop_invoice', fields: {} } },
+      // Unreadable in the other shape the arbiter names.
+      broken_array: { type: 'master_detail', reference: ['shop_invoice'] },
+      // Absence is legal and stays silent: `.optional()` admits it.
+      untargeted: { type: 'lookup' },
+      // Not a reference-typed field at all.
+      title: { type: 'text' },
+    }),
+    logger: { info() {}, warn, error() {}, debug() {} },
+  });
+  // `resolveLookupFields` is private and has no public seam: its sole consumer
+  // is the inbox display enrichment, which swallows every failure by design.
+  // Reading it directly is what makes the manufactured name assertable at all.
+  const resolve = (object: string) =>
+    (service as unknown as { resolveLookupFields(o: string): Array<{ key: string; reference: string }> })
+      .resolveLookupFields(object);
+  return { resolve, warn };
+};
+
+describe('ApprovalService.resolveLookupFields — an unreadable `reference` carrier', () => {
+  it('never manufactures the literal target name `[object Object]`', () => {
+    const { resolve } = makeService();
+    const references = resolve('deal').map(f => f.reference);
+    expect(references).not.toContain('[object Object]');
+    // The general form of the same claim: nothing a `String()` of a non-string
+    // could have produced survives into the result.
+    for (const reference of references) {
+      expect(typeof reference).toBe('string');
+      expect(reference).not.toMatch(/^\[object /);
+    }
+  });
+
+  it('leaves the unreadable fields out and still carries the readable one', () => {
+    const { resolve } = makeService();
+    expect(resolve('deal')).toEqual([{ key: 'account', reference: 'crm_account' }]);
+  });
+
+  it('reports each unreadable carrier instead of dropping it silently', () => {
+    const { resolve, warn } = makeService();
+    resolve('deal');
+    const messages = warn.mock.calls.map(args => String(args[0]));
+    expect(messages).toHaveLength(2);
+    expect(messages.some(m => m.includes('deal.broken'))).toBe(true);
+    expect(messages.some(m => m.includes('deal.broken_array'))).toBe(true);
+    // The refusal names the reader and says what to write instead.
+    expect(messages.every(m => m.includes('ApprovalService.resolveLookupFields'))).toBe(true);
+  });
+
+  it('stays silent for a field that legitimately names no target', () => {
+    const { resolve, warn } = makeService();
+    resolve('deal');
+    expect(warn.mock.calls.every(args => !String(args[0]).includes('untargeted'))).toBe(true);
+  });
+});