Skip to content

Commit 0f6b98b

Browse files
committed
test(reference-carrier): pin absence at all four readers, with the readable control
Each site gets a case that fails before the narrowing — the approvals one pins the literal `[object Object]` directly, since that string is the whole evidence — and a control that keeps a readable string target flowing, so "narrowed" and "this path is closed" stay distinguishable. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
1 parent 2d26b63 commit 0f6b98b

4 files changed

Lines changed: 241 additions & 1 deletion

File tree

‎packages/cli/src/commands/doctor.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,9 @@ export function resolveTenancyPostureOrFinding(reading: DotenvReading): TenancyP
701701

702702
// ─── Config-Aware Checks ────────────────────────────────────────────
703703

704-
function detectCircularDependencies(objects: any[]): string[] {
704+
// Exported for the pin on its carrier reading below; `doctor` itself is the
705+
// only caller.
706+
export function detectCircularDependencies(objects: any[]): string[] {
705707
const issues: string[] = [];
706708
const graph = new Map<string, string[]>();
707709

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license.
2+
//
3+
// Two of `os doctor`'s config checks read a lookup field's target through a
4+
// truthiness gate — `if (field?.type === 'lookup' && field?.reference)` — and
5+
// then put the value straight into a graph node (`detectCircularDependencies`)
6+
// or a name set (`findUnusedObjects`). An object- or array-valued `reference`
7+
// passes truthiness, so a non-string entered both, where it matches no object
8+
// name and renders as `[object Object]` in a cycle message.
9+
//
10+
// `collectViewObjectRefs`, twenty lines away in the same file, already narrowed
11+
// its carrier with `typeof … === 'string'`; these two now read it through the
12+
// same arbiter the rest of the platform uses. Unreadability is REPORTED rather
13+
// than skipped, because both checks publish a positive verdict — "no circular
14+
// references detected", "defined but not referenced" — that an edge nobody
15+
// could read cannot support.
16+
17+
import { describe, it, expect } from 'vitest';
18+
import { detectCircularDependencies, findUnusedObjects } from '../src/commands/doctor';
19+
20+
/** An `ObjectSchema` literal where the target NAME belongs. */
21+
const UNREADABLE = { name: 'crm_account', fields: {} };
22+
23+
const obj = (name: string, fields: Record<string, unknown>) => ({ name, label: name, fields });
24+
25+
describe('doctor.detectCircularDependencies — an unreadable `reference` carrier', () => {
26+
it('never puts a non-string into the dependency graph, and says so', () => {
27+
const issues = detectCircularDependencies([
28+
obj('crm_contact', { account: { type: 'lookup', reference: UNREADABLE } }),
29+
obj('crm_account', {}),
30+
]);
31+
expect(issues.join('\n')).not.toContain('[object Object]');
32+
expect(issues).toHaveLength(1);
33+
expect(issues[0]).toContain('crm_contact');
34+
expect(issues[0]).toContain('account');
35+
expect(issues[0]).toContain('doctor.detectCircularDependencies');
36+
});
37+
38+
it('still detects a cycle built from readable targets', () => {
39+
const issues = detectCircularDependencies([
40+
obj('crm_contact', { account: { type: 'lookup', reference: 'crm_account' } }),
41+
obj('crm_account', { primary_contact: { type: 'lookup', reference: 'crm_contact' } }),
42+
]);
43+
expect(issues).toHaveLength(1);
44+
expect(issues[0]).toContain('Circular dependency');
45+
});
46+
47+
it('stays silent for a lookup that legitimately names no target', () => {
48+
expect(detectCircularDependencies([obj('crm_contact', { account: { type: 'lookup' } })])).toEqual([]);
49+
});
50+
});
51+
52+
describe('doctor.findUnusedObjects — an unreadable `reference` carrier', () => {
53+
const config = (reference: unknown) => ({
54+
objects: [
55+
obj('crm_contact', { account: { type: 'lookup', reference } }),
56+
obj('crm_account', { title: { type: 'text' } }),
57+
],
58+
views: [{ list: { type: 'grid', data: { provider: 'object', object: 'crm_contact' } } }],
59+
});
60+
61+
it('reports the unreadable carrier rather than letting it distort the verdict', () => {
62+
const found = findUnusedObjects(config(UNREADABLE));
63+
expect(found.join('\n')).not.toContain('[object Object]');
64+
// The carrier finding is present, and names the field that carries it.
65+
expect(found.some(m => m.includes('crm_contact') && m.includes('account') && m.includes('unreadable')))
66+
.toBe(true);
67+
expect(found.some(m => m.includes('doctor.findUnusedObjects'))).toBe(true);
68+
});
69+
70+
it('still counts a readable lookup target as a reference', () => {
71+
// `crm_account` is referenced ONLY by the lookup, so this is the control
72+
// that separates "narrowed" from "this path was closed".
73+
expect(findUnusedObjects(config('crm_account'))).toEqual([]);
74+
});
75+
});
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license.
2+
//
3+
// `resolveLookupFields` used to gate the carrier with a truthiness test and
4+
// then stringify it: `out.push({ key, reference: String(f.reference) })`. An
5+
// object-valued `reference` passes truthiness and `String()` renders it as the
6+
// literal target name `[object Object]` — a name that can never resolve, handed
7+
// on to `engine.find(<object name>)` by the inbox display enrichment and lost
8+
// inside that caller's own `catch`.
9+
//
10+
// `FieldSchema.reference` is declared an optional STRING, so the answer a
11+
// reader owes for an unreadable carrier is ABSENCE. These cases pin both
12+
// halves: the unreadable field is left out and reported, and a readable one is
13+
// still carried through unchanged — otherwise "fixed" and "this path is now
14+
// closed" would be indistinguishable.
15+
16+
import { describe, it, expect, vi } from 'vitest';
17+
import { ApprovalService } from './approval-service.js';
18+
19+
/** The one engine member `resolveLookupFields` reads. */
20+
const engineWithSchema = (fields: Record<string, unknown>) => ({
21+
getSchema: (_object: string) => ({ fields }),
22+
}) as any;
23+
24+
const makeService = () => {
25+
const warn = vi.fn();
26+
const service = new ApprovalService({
27+
engine: engineWithSchema({
28+
// Readable: the control that keeps this a narrowing rather than a shutdown.
29+
account: { type: 'lookup', reference: 'crm_account' },
30+
// Unreadable: an `ObjectSchema` literal where the target NAME belongs.
31+
// `ObjectSchema.safeParse` refuses this at the contract door, so a value
32+
// in this shape reached the reader without ever passing parse.
33+
broken: { type: 'lookup', reference: { name: 'shop_invoice', fields: {} } },
34+
// Unreadable in the other shape the arbiter names.
35+
broken_array: { type: 'master_detail', reference: ['shop_invoice'] },
36+
// Absence is legal and stays silent: `.optional()` admits it.
37+
untargeted: { type: 'lookup' },
38+
// Not a reference-typed field at all.
39+
title: { type: 'text' },
40+
}),
41+
logger: { info() {}, warn, error() {}, debug() {} },
42+
});
43+
// `resolveLookupFields` is private and has no public seam: its sole consumer
44+
// is the inbox display enrichment, which swallows every failure by design.
45+
// Reading it directly is what makes the manufactured name assertable at all.
46+
const resolve = (object: string) =>
47+
(service as unknown as { resolveLookupFields(o: string): Array<{ key: string; reference: string }> })
48+
.resolveLookupFields(object);
49+
return { resolve, warn };
50+
};
51+
52+
describe('ApprovalService.resolveLookupFields — an unreadable `reference` carrier', () => {
53+
it('never manufactures the literal target name `[object Object]`', () => {
54+
const { resolve } = makeService();
55+
const references = resolve('deal').map(f => f.reference);
56+
expect(references).not.toContain('[object Object]');
57+
// The general form of the same claim: nothing a `String()` of a non-string
58+
// could have produced survives into the result.
59+
for (const reference of references) {
60+
expect(typeof reference).toBe('string');
61+
expect(reference).not.toMatch(/^\[object /);
62+
}
63+
});
64+
65+
it('leaves the unreadable fields out and still carries the readable one', () => {
66+
const { resolve } = makeService();
67+
expect(resolve('deal')).toEqual([{ key: 'account', reference: 'crm_account' }]);
68+
});
69+
70+
it('reports each unreadable carrier instead of dropping it silently', () => {
71+
const { resolve, warn } = makeService();
72+
resolve('deal');
73+
const messages = warn.mock.calls.map(args => String(args[0]));
74+
expect(messages).toHaveLength(2);
75+
expect(messages.some(m => m.includes('deal.broken'))).toBe(true);
76+
expect(messages.some(m => m.includes('deal.broken_array'))).toBe(true);
77+
// The refusal names the reader and says what to write instead.
78+
expect(messages.every(m => m.includes('ApprovalService.resolveLookupFields'))).toBe(true);
79+
});
80+
81+
it('stays silent for a field that legitimately names no target', () => {
82+
const { resolve, warn } = makeService();
83+
resolve('deal');
84+
expect(warn.mock.calls.every(args => !String(args[0]).includes('untargeted'))).toBe(true);
85+
});
86+
});
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license.
2+
//
3+
// ADR-0021's relationship → target-object resolver used to gate the carrier
4+
// with a truthiness test and return it: `if (… && field.reference) return
5+
// field.reference`. An object- or array-valued `reference` passes that gate, so
6+
// the JOINED TABLE for a dataset's `include` became a non-string — a value the
7+
// declared contract (`FieldSchema.reference`, an optional STRING) never admits.
8+
//
9+
// The resolver's answer for a carrier no reader can read is now ABSENCE, which
10+
// the resolver's own fallback turns into the compiler's "cannot resolve this
11+
// relationship" refusal, plus one warning naming the field. The readable case
12+
// is pinned alongside it, so a narrowing cannot be mistaken for a shutdown.
13+
14+
import { describe, it, expect, vi } from 'vitest';
15+
import { AnalyticsServicePlugin } from '../plugin.js';
16+
17+
type Resolver = (baseObject: string, relationshipName: string) => string | undefined;
18+
19+
const OBJECTS: Record<string, { fields: Record<string, unknown> }> = {
20+
shop_order: {
21+
fields: {
22+
// Readable — the control.
23+
account: { type: 'lookup', reference: 'crm_account' },
24+
// Unreadable: an object where the target NAME belongs. Refused by
25+
// `ObjectSchema.safeParse`, so it only ever arrives unparsed.
26+
broken: { type: 'lookup', reference: { name: 'shop_invoice', fields: {} } },
27+
// Unreadable, array shape.
28+
broken_array: { type: 'master_detail', reference: ['shop_invoice'] },
29+
// Absence is legal and stays silent.
30+
untargeted: { type: 'lookup' },
31+
},
32+
},
33+
};
34+
35+
async function bootResolver() {
36+
const warn = vi.fn();
37+
const registered: Record<string, unknown> = {};
38+
const ctx = {
39+
getService: (name: string) =>
40+
name === 'data'
41+
? { getObject: (o: string) => OBJECTS[o], aggregate: async () => [], execute: async () => ({ rows: [] }) }
42+
: registered[name],
43+
registerService: (name: string, svc: unknown) => { registered[name] = svc; },
44+
replaceService: (name: string, svc: unknown) => { registered[name] = svc; },
45+
logger: { info() {}, warn, error() {}, debug() {} },
46+
};
47+
await new AnalyticsServicePlugin().init(ctx as never);
48+
// The resolver is a closure built inside `init` and handed to the service as
49+
// config; the service's own field is the only handle on it. Driving a whole
50+
// dataset compile would add scaffolding without changing what is measured —
51+
// which value this function returns for an unreadable carrier.
52+
const resolver = (registered.analytics as unknown as { relationshipResolver: Resolver }).relationshipResolver;
53+
return { resolver, warn };
54+
}
55+
56+
describe('analytics relationship resolver — an unreadable `reference` carrier', () => {
57+
it('answers `undefined` instead of a non-string joined table', async () => {
58+
const { resolver } = await bootResolver();
59+
expect(resolver('shop_order', 'broken')).toBeUndefined();
60+
expect(resolver('shop_order', 'broken_array')).toBeUndefined();
61+
});
62+
63+
it('still resolves a readable target', async () => {
64+
const { resolver } = await bootResolver();
65+
expect(resolver('shop_order', 'account')).toBe('crm_account');
66+
});
67+
68+
it('reports the unreadable carrier, and stays silent for a legal absence', async () => {
69+
const { resolver, warn } = await bootResolver();
70+
resolver('shop_order', 'untargeted');
71+
expect(warn.mock.calls.filter(args => String(args[0]).includes('untargeted'))).toHaveLength(0);
72+
resolver('shop_order', 'broken');
73+
const messages = warn.mock.calls.map(args => String(args[0]));
74+
expect(messages.some(m => m.includes('shop_order.broken'))).toBe(true);
75+
expect(messages.some(m => m.includes('Analytics.relationshipResolver'))).toBe(true);
76+
});
77+
});

0 commit comments

Comments
 (0)