Skip to content

Commit b58ca1a

Browse files
committed
feat(objectql): the silent skips stop being silent — registration-time completeness diagnostics (ADR-0078 Phase 4)
Phase 4 had two halves; they got opposite verdicts, decided on evidence already in hand rather than deferred for evidence that will never arrive. ## Rejected — the generative rule sweep (not deferred: rejected) A generator can enumerate candidates ("which optional keys might be load-bearing?") but cannot verify runtime skip sites, and a rule without its skip-site citation is a false prescription. This campaign shipped four of those; every one was caught by the verification pass a generator would structurally skip. No amount of waiting fixes that — the route is wrong, not early. ## Built — registration-time diagnostics at the choke point every door shares The author-time gate (validate-functional-completeness) only protects metadata that passes through `os build` / `validate` / `lint`. Two shipped instances prove the other doors are real: #3896 (Setup authoring inserted `sys_sharing_rule` rows directly, bypassing the schema that "required" `criteria`) and cloud's `rowColor.mapping` (an `as never` cast bypassed tsc, then the strip-era parse dropped the key). `SchemaRegistry.registerObject` is where EVERY door converges — declared stacks, plugin objects, `extend` contributions, `saveMetaItem`, raw registerObject calls. It now runs the SAME shared predicate (`checkFieldCompleteness` from `@objectstack/spec/kernel`) and emits one aggregated warning per incomplete object, carrying the SAME rule ids the lint reports — a boot log greps straight into the same docs and suppression story. WARN, never throw — deliberately: ADR-0078 §1 maps error severity to "the INSTANCE is dead", not "the system is dead". An inert field must not kill a boot that thousands of healthy objects share. Errors block at author time; the registry's job is that the silence never survives to runtime unobserved. Shape follows `warnStrippedLegacyApiMethods` (#3543) exactly: module-level once-per-object dedup, injectable `warn`, pure observation, hot path untouched. ## The webhook skip now names itself `auto-enqueuer.ts`'s `if (triggers.size === 0) return null` sat under a comment blessing the empty case as "a manual-only webhook" — a mode #3196 removed (no manual fire path exists). The skip now warns with the author-time rule id (`webhook/without-triggers`) and the comment tells the truth. Verified, not assumed: only ACTIVE rows reach parseRow (`where: { active: true }`), so a deliberately disabled webhook stays warning-free — zero false positives on the repo's one real webhook (shipped inactive, full trigger list). ## Scope honesty Field rules + the webhook rule get the runtime twin. `view/layout-without- binding` stays author-time-only: views do not register through this choke point and the renderer half of the evidence lives in objectui. ## Verification - registry 85/85 (6 new, incl. an integration test through a raw registerObject call — the #3896 class of door) - auto-enqueuer 15/15 (1 new) - Full suite 132/132. The dogfood boots double as the authoritative sweep: every shipped stack (platform objects, showcase, CRM, Todo) registers with ZERO functionally-incomplete warnings — no false positives, and no inert fields hiding behind doors the author-time gate could not see. Closes the ADR-0078 loop end to end: author-time error, runtime warning, one shared predicate deciding both. Tracked in #4544. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WnqGjQFQMqd5k81LYV8SCY
1 parent d91d39e commit b58ca1a

5 files changed

Lines changed: 235 additions & 4 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/objectql': minor
3+
'@objectstack/plugin-webhooks': patch
4+
---
5+
6+
ADR-0078 Phase 4, decided rather than deferred: the silent skips stop being silent at runtime. The registry — the one choke point every metadata door goes through — now emits a functional-completeness diagnostic at registration, and the webhook enqueuer's zero-trigger skip warns instead of returning `null` wordlessly.
7+
8+
**The Phase 4 ruling.** The phase had two halves, and they got opposite verdicts:
9+
10+
- **Generative rule sweep: rejected — not deferred.** A generator can enumerate candidates ("which optional keys might be load-bearing?") but cannot verify runtime skip sites, and a rule without its skip-site citation is a false prescription — this campaign shipped four of those and every one was caught by the verification pass a generator would skip. The route is structurally wrong; no amount of waiting produces the evidence that would fix it.
11+
- **Registration-time diagnostics: built now.** The evidence was already in hand, not pending: #3896 (Setup authoring inserted `sys_sharing_rule` rows directly, bypassing the schema that "required" `criteria`) and cloud's `rowColor.mapping` (an `as never` cast bypassed tsc) prove that doors which skip Zod and lint are real. The author-time gate only protects metadata that passes through `os build` / `validate` / `lint`; `SchemaRegistry.registerObject` is where *every* door converges — declared stacks, plugin objects, `extend` contributions, `saveMetaItem`, raw `registerObject` calls.
12+
13+
**Same predicate, same rule ids, different posture.** The registry calls the same `checkFieldCompleteness` that `validate-functional-completeness` uses, so the boot log carries the *same rule ids* the lint reports (`field/summary-without-operations`, …) — an operator or an AI reading the log greps the id straight into the same docs and suppression story. But the registry **warns and never throws**: ADR-0078 §1's error severity means *the instance is dead*, not *the system is dead* — an inert field must not kill a boot that thousands of healthy objects share. Errors block at author time; the registry's job is to make sure the silence never survives to runtime unobserved.
14+
15+
One line per object with every finding aggregated (not per request — the hot path stays free; not per finding — a three-dead-field object is one greppable line). Follows `warnStrippedLegacyApiMethods` (#3543) exactly: module-level once-per-object dedup, injectable `warn`, pure observation that never mutates the schema.
16+
17+
**The webhook skip now names itself.** `auto-enqueuer.ts`'s `if (triggers.size === 0) return null` sat under a comment blessing the empty case as "a manual-only webhook" — a mode #3196 removed (no manual fire path exists). The skip now warns with the author-time rule id (`webhook/without-triggers`), and the comment tells the truth. Only *active* rows reach the parse (`where: { active: true }` — verified, not assumed), so a deliberately disabled webhook stays warning-free.
18+
19+
**Scope honesty:** field rules and the webhook rule get the runtime twin. `view/layout-without-binding` stays author-time-only — views don't register through this choke point and the renderer half of the evidence lives in objectui.
20+
21+
Tracked in #4544. This closes the ADR-0078 loop end to end: author-time error, runtime warning, one shared predicate deciding both.

packages/objectql/src/registry.test.ts

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, beforeEach, vi } from 'vitest';
2-
import { SchemaRegistry, applySystemFields, reconcileManagedApiMethods, warnStrippedLegacyApiMethods, computeFQN, parseFQN } from './registry';
2+
import { SchemaRegistry, applySystemFields, reconcileManagedApiMethods, warnStrippedLegacyApiMethods, warnFunctionalCompleteness, computeFQN, parseFQN } from './registry';
33
import { AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data';
44

55
describe('SchemaRegistry', () => {
@@ -924,3 +924,102 @@ describe('warnStrippedLegacyApiMethods (#3543)', () => {
924924
expect(warn).toHaveBeenCalledTimes(1);
925925
});
926926
});
927+
928+
// ==========================================
929+
// warnFunctionalCompleteness — ADR-0078 Phase 4
930+
// Registration-time twin of `validate-functional-completeness`: the registry
931+
// is the one choke point every metadata door goes through, including the ones
932+
// that skip Zod and lint (#3896, raw registerObject). Same shared predicate,
933+
// same rule ids. Pure observation — never mutates the schema, never throws.
934+
// ==========================================
935+
describe('warnFunctionalCompleteness (ADR-0078 Phase 4)', () => {
936+
it('diagnoses a bare summary field with the SAME rule id the lint reports', () => {
937+
const warn = vi.fn();
938+
warnFunctionalCompleteness(
939+
{ name: 'fc_room', fields: { registration_count: { type: 'summary', label: 'Registrations' } } } as any,
940+
{ warn },
941+
);
942+
expect(warn).toHaveBeenCalledTimes(1);
943+
const msg = warn.mock.calls[0][0] as string;
944+
expect(msg).toContain('fc_room');
945+
expect(msg).toContain('registration_count');
946+
expect(msg).toContain('field/summary-without-operations');
947+
expect(msg).toContain('ADR-0078');
948+
// The prescription rides along — a warning with no fix is a dead end.
949+
expect(msg).toContain('summaryOperations');
950+
});
951+
952+
it('aggregates every inert field into ONE line (greppable, not spam)', () => {
953+
const warn = vi.fn();
954+
warnFunctionalCompleteness(
955+
{
956+
name: 'fc_multi',
957+
fields: {
958+
total: { type: 'summary' },
959+
rate: { type: 'formula' },
960+
acct: { type: 'lookup' },
961+
ok: { type: 'text', label: 'Fine' },
962+
},
963+
} as any,
964+
{ warn },
965+
);
966+
expect(warn).toHaveBeenCalledTimes(1);
967+
const msg = warn.mock.calls[0][0] as string;
968+
expect(msg).toContain('field/summary-without-operations');
969+
expect(msg).toContain('field/formula-without-expression');
970+
expect(msg).toContain('field/relationship-without-reference');
971+
expect(msg).not.toContain('" ok:'); // the healthy field is not named
972+
});
973+
974+
it('stays silent for a complete schema — the predicate decides, not this wrapper', () => {
975+
const warn = vi.fn();
976+
warnFunctionalCompleteness(
977+
{
978+
name: 'fc_clean',
979+
fields: {
980+
total: { type: 'summary', summaryOperations: { object: 'line', field: 'amt', function: 'sum' } },
981+
acct: { type: 'lookup', reference: 'account' },
982+
stage: { type: 'select', options: [{ label: 'New', value: 'new' }] },
983+
tags: { type: 'multiselect' }, // the pinned NON-rule stays a NON-rule here too
984+
},
985+
} as any,
986+
{ warn },
987+
);
988+
expect(warn).not.toHaveBeenCalled();
989+
});
990+
991+
it('stays silent for fieldless / malformed schemas (never the thing that crashes a boot)', () => {
992+
const warn = vi.fn();
993+
warnFunctionalCompleteness({ name: 'fc_nofields' } as any, { warn });
994+
warnFunctionalCompleteness({ name: 'fc_badfields', fields: 'nope' } as any, { warn });
995+
expect(warn).not.toHaveBeenCalled();
996+
});
997+
998+
it('warns only once per object name (hot path stays free)', () => {
999+
const warn = vi.fn();
1000+
const schema: any = { name: 'fc_once', fields: { t: { type: 'summary' } } };
1001+
warnFunctionalCompleteness(schema, { warn });
1002+
warnFunctionalCompleteness(schema, { warn });
1003+
expect(warn).toHaveBeenCalledTimes(1);
1004+
});
1005+
1006+
it('fires through registerObject — the choke point every door shares', () => {
1007+
// The integration half: a raw registerObject call (no Zod, no lint —
1008+
// the #3896 class of door) still gets the diagnostic.
1009+
const spy = vi.spyOn(console, 'warn').mockImplementation(() => {});
1010+
try {
1011+
const registry = new SchemaRegistry();
1012+
registry.registerObject(
1013+
{ name: 'fc_via_register', label: 'X', fields: { dead: { type: 'formula' } } } as any,
1014+
'test-pkg',
1015+
);
1016+
const hit = spy.mock.calls.find(
1017+
(c) => typeof c[0] === 'string' && (c[0] as string).includes('fc_via_register'),
1018+
);
1019+
expect(hit).toBeDefined();
1020+
expect(hit![0]).toContain('field/formula-without-expression');
1021+
} finally {
1022+
spy.mockRestore();
1023+
}
1024+
});
1025+
});

packages/objectql/src/registry.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary, resolveCrudAffordances, isTenancyDisabled, LEGACY_API_METHODS, AUDIT_PROVENANCE_FIELDS, type AuditProvenanceField } from '@objectstack/spec/data';
44
import { resolveMultiOrgEnabled, resolveSearchPinyinEnabled } from '@objectstack/types';
55
import { provisionSearchCompanion } from './search-companion.js';
6-
import { ObjectStackManifest, ManifestSchema, InstalledPackage, InstalledPackageSchema } from '@objectstack/spec/kernel';
6+
import { ObjectStackManifest, ManifestSchema, InstalledPackage, InstalledPackageSchema, checkFieldCompleteness } from '@objectstack/spec/kernel';
77
import { AppSchema } from '@objectstack/spec/ui';
88
import { applyProtection } from '@objectstack/spec/shared';
99

@@ -583,6 +583,66 @@ export function warnStrippedLegacyApiMethods(
583583
);
584584
}
585585

586+
/** Objects already diagnosed for functional completeness (once per object). */
587+
const warnedFunctionalCompleteness = new Set<string>();
588+
589+
/**
590+
* [ADR-0078 Phase 4] Registration-time functional-completeness diagnostic.
591+
*
592+
* The author-time gate (`@objectstack/lint`'s `validate-functional-completeness`)
593+
* only protects metadata that passes through `os build` / `validate` / `lint`.
594+
* The registry is the one choke point EVERY door goes through — declared
595+
* stacks, plugin-provided objects, `extend` contributions, `saveMetaItem`, raw
596+
* `registerObject` calls — including the doors that skip Zod and lint entirely.
597+
* Two shipped instances prove those doors are real, not hypothetical: #3896
598+
* (Setup authoring inserted `sys_sharing_rule` rows directly, bypassing the
599+
* schema that "required" `criteria`) and cloud's `rowColor.mapping` (an
600+
* `as never` cast bypassed tsc, then the strip-era parse dropped the key).
601+
*
602+
* Same shared predicate as the author-time gate (`checkFieldCompleteness` in
603+
* `@objectstack/spec/kernel`), so the rule ids in this warning are the SAME ids
604+
* the lint reports — an operator or an AI reading the boot log can grep the id
605+
* straight into the docs and the suppression story. Judgement lives only in the
606+
* predicate; if a rule seems wrong, fix it there, never here.
607+
*
608+
* WARN, never throw — deliberately, and not as a soft default: an inert field
609+
* must not kill a boot that thousands of healthy objects share (ADR-0078 §1
610+
* maps error-severity to "the INSTANCE is dead", not "the system is dead").
611+
* The author-time gate is where errors block; the registry's job is to make
612+
* sure the silence never survives to runtime unobserved.
613+
*
614+
* Emitted once per object name with every finding aggregated into that one
615+
* line (not per request, not per finding — the hot path stays free and a
616+
* 3-dead-field object is one greppable line, not three).
617+
*/
618+
export function warnFunctionalCompleteness(
619+
schema: ServiceObject,
620+
opts?: { warn?: (msg: string) => void },
621+
): void {
622+
const fields = (schema as { fields?: Record<string, unknown> }).fields;
623+
if (!fields || typeof fields !== 'object') return;
624+
const name = String((schema as { name?: unknown }).name ?? '');
625+
if (warnedFunctionalCompleteness.has(name)) return;
626+
627+
const findings: string[] = [];
628+
for (const [fieldName, def] of Object.entries(fields)) {
629+
for (const f of checkFieldCompleteness(def)) {
630+
findings.push(`${fieldName}: [${f.severity}] ${f.rule} — add \`${f.fix}\``);
631+
}
632+
}
633+
if (findings.length === 0) return;
634+
warnedFunctionalCompleteness.add(name);
635+
636+
const warn = opts?.warn ?? ((msg: string) => console.warn(msg));
637+
warn(
638+
`[Registry] Object "${name}" registered with ${findings.length} functionally-incomplete ` +
639+
`field(s) — Zod-valid but runtime-DEAD: the consumer silently skips each one, so it reads ` +
640+
`0/null/never-resolves while every authoring surface reports success (ADR-0078). ` +
641+
findings.join(' · ') +
642+
` — \`os lint\` reports the same rule ids with full context.`,
643+
);
644+
}
645+
586646
/**
587647
* Platform namespaces that multiple packages may legitimately share, so the
588648
* install-time namespace-uniqueness gate (ADR-0048 Phase 1) must never fire on
@@ -878,6 +938,13 @@ export class SchemaRegistry {
878938
// reconcile so we diagnose what actually ships.
879939
warnStrippedLegacyApiMethods(schema);
880940

941+
// [ADR-0078 Phase 4] One-shot per-object functional-completeness
942+
// diagnostic — the registry is the choke point every metadata door goes
943+
// through, including the ones that skip Zod and lint (#3896, raw
944+
// registerObject). Same shared predicate and rule ids as `os lint`;
945+
// warn-never-throw (an inert FIELD must not kill the boot).
946+
warnFunctionalCompleteness(schema);
947+
881948
// [ADR-0079] Object-materialization seam — DESIGNATE-ONLY primary-title
882949
// provisioning. Runs AFTER `applySystemFields` (so any designated field
883950
// co-exists with the injected system columns) and ONLY for owned objects

packages/plugins/plugin-webhooks/src/auto-enqueuer.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,33 @@ describe('AutoEnqueuer', () => {
329329
await ae.stop();
330330
});
331331

332+
it('says OUT LOUD that a zero-trigger webhook will never fire (ADR-0078 Phase 4)', async () => {
333+
// The skip used to be silent, under a comment blessing it as "a
334+
// manual-only webhook" — a mode #3196 removed (no manual fire path
335+
// exists). A zero-trigger ACTIVE row is a dead subscription that looks
336+
// armed in Setup, so the skip now warns with the same rule id the
337+
// author-time gate reports (`webhook/without-triggers`). Inactive rows
338+
// never reach parseRow (the cache query filters `active: true`), so a
339+
// deliberately-disabled webhook stays warning-free.
340+
const engine = new FakeEngine({ sys_webhook: [webhook({ triggers: '' })] });
341+
const realtime = new FakeRealtime();
342+
const { enqueue, calls } = makeRecorder();
343+
const warn = vi.fn();
344+
const ae = new AutoEnqueuer(engine, realtime, enqueue, { refreshIntervalMs: 0, logger: { warn } });
345+
await ae.start();
346+
347+
await realtime.publish(event('created', 'contact', { id: 'c-1' }));
348+
await flush();
349+
350+
expect(calls).toHaveLength(0);
351+
expect(warn).toHaveBeenCalledWith(
352+
expect.stringContaining('webhook/without-triggers'),
353+
expect.objectContaining({ id: 'wh-1' }),
354+
);
355+
expect(String(warn.mock.calls[0][0])).toContain('NEVER fire');
356+
await ae.stop();
357+
});
358+
332359
it('self-heals the cache when sys_webhook changes', async () => {
333360
const engine = new FakeEngine({ sys_webhook: [] });
334361
const realtime = new FakeRealtime();

packages/plugins/plugin-webhooks/src/auto-enqueuer.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,25 @@ export class AutoEnqueuer {
244244
normalized.filter((t) => DISPATCHABLE_WEBHOOK_TRIGGERS.has(t)) as Array<'create' | 'update' | 'delete'>,
245245
);
246246
if (triggers.size === 0) {
247-
// No dispatchable triggers (or a manual-only webhook with none) —
248-
// skip auto-enqueue.
247+
// [ADR-0078 Phase 4] No dispatchable triggers — the webhook can
248+
// never fire on ANY path, so say so instead of skipping silently.
249+
// This comment used to read "(or a manual-only webhook with
250+
// none)", but that mode does not exist: the `api` trigger was
251+
// REMOVED (#3196, `webhook.zod.ts`) precisely because there is no
252+
// manual fire path — the only webhook HTTP surface re-queues
253+
// already-failed deliveries. So a zero-trigger row is not an off
254+
// switch (that is `active`), it is a dead subscription that looks
255+
// armed in Setup. Same rule id as the author-time gate
256+
// (`webhook/without-triggers`) so the boot log greps into the
257+
// same docs. Only active rows reach parseRow, so a deliberately
258+
// disabled webhook stays warning-free.
259+
this.logger.warn?.(
260+
`[webhook-auto-enqueuer] webhook '${(row.name as string) ?? row.id}' has no dispatchable ` +
261+
`triggers — it will NEVER fire (rule webhook/without-triggers): there is no manual fire ` +
262+
`path (#3196), so this row is dead while looking armed in Setup. Declare ` +
263+
`triggers: ['create'|'update'|'delete'], or set it inactive if it should be off.`,
264+
{ id: row.id },
265+
);
249266
return null;
250267
}
251268

0 commit comments

Comments
 (0)