diff --git a/.changeset/adr-0078-phase4-runtime-warns.md b/.changeset/adr-0078-phase4-runtime-warns.md new file mode 100644 index 0000000000..2f30946f91 --- /dev/null +++ b/.changeset/adr-0078-phase4-runtime-warns.md @@ -0,0 +1,21 @@ +--- +'@objectstack/objectql': minor +'@objectstack/plugin-webhooks': patch +--- + +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. + +**The Phase 4 ruling.** The phase had two halves, and they got opposite verdicts: + +- **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. +- **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. + +**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. + +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. + +**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. + +**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. + +Tracked in #4544. This closes the ADR-0078 loop end to end: author-time error, runtime warning, one shared predicate deciding both. diff --git a/packages/objectql/src/registry.test.ts b/packages/objectql/src/registry.test.ts index 297df834c1..3708d07d97 100644 --- a/packages/objectql/src/registry.test.ts +++ b/packages/objectql/src/registry.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { SchemaRegistry, applySystemFields, reconcileManagedApiMethods, warnStrippedLegacyApiMethods, computeFQN, parseFQN } from './registry'; +import { SchemaRegistry, applySystemFields, reconcileManagedApiMethods, warnStrippedLegacyApiMethods, warnFunctionalCompleteness, computeFQN, parseFQN } from './registry'; import { AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data'; describe('SchemaRegistry', () => { @@ -924,3 +924,102 @@ describe('warnStrippedLegacyApiMethods (#3543)', () => { expect(warn).toHaveBeenCalledTimes(1); }); }); + +// ========================================== +// warnFunctionalCompleteness — ADR-0078 Phase 4 +// Registration-time twin of `validate-functional-completeness`: the registry +// is the one choke point every metadata door goes through, including the ones +// that skip Zod and lint (#3896, raw registerObject). Same shared predicate, +// same rule ids. Pure observation — never mutates the schema, never throws. +// ========================================== +describe('warnFunctionalCompleteness (ADR-0078 Phase 4)', () => { + it('diagnoses a bare summary field with the SAME rule id the lint reports', () => { + const warn = vi.fn(); + warnFunctionalCompleteness( + { name: 'fc_room', fields: { registration_count: { type: 'summary', label: 'Registrations' } } } as any, + { warn }, + ); + expect(warn).toHaveBeenCalledTimes(1); + const msg = warn.mock.calls[0][0] as string; + expect(msg).toContain('fc_room'); + expect(msg).toContain('registration_count'); + expect(msg).toContain('field/summary-without-operations'); + expect(msg).toContain('ADR-0078'); + // The prescription rides along — a warning with no fix is a dead end. + expect(msg).toContain('summaryOperations'); + }); + + it('aggregates every inert field into ONE line (greppable, not spam)', () => { + const warn = vi.fn(); + warnFunctionalCompleteness( + { + name: 'fc_multi', + fields: { + total: { type: 'summary' }, + rate: { type: 'formula' }, + acct: { type: 'lookup' }, + ok: { type: 'text', label: 'Fine' }, + }, + } as any, + { warn }, + ); + expect(warn).toHaveBeenCalledTimes(1); + const msg = warn.mock.calls[0][0] as string; + expect(msg).toContain('field/summary-without-operations'); + expect(msg).toContain('field/formula-without-expression'); + expect(msg).toContain('field/relationship-without-reference'); + expect(msg).not.toContain('" ok:'); // the healthy field is not named + }); + + it('stays silent for a complete schema — the predicate decides, not this wrapper', () => { + const warn = vi.fn(); + warnFunctionalCompleteness( + { + name: 'fc_clean', + fields: { + total: { type: 'summary', summaryOperations: { object: 'line', field: 'amt', function: 'sum' } }, + acct: { type: 'lookup', reference: 'account' }, + stage: { type: 'select', options: [{ label: 'New', value: 'new' }] }, + tags: { type: 'multiselect' }, // the pinned NON-rule stays a NON-rule here too + }, + } as any, + { warn }, + ); + expect(warn).not.toHaveBeenCalled(); + }); + + it('stays silent for fieldless / malformed schemas (never the thing that crashes a boot)', () => { + const warn = vi.fn(); + warnFunctionalCompleteness({ name: 'fc_nofields' } as any, { warn }); + warnFunctionalCompleteness({ name: 'fc_badfields', fields: 'nope' } as any, { warn }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('warns only once per object name (hot path stays free)', () => { + const warn = vi.fn(); + const schema: any = { name: 'fc_once', fields: { t: { type: 'summary' } } }; + warnFunctionalCompleteness(schema, { warn }); + warnFunctionalCompleteness(schema, { warn }); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('fires through registerObject — the choke point every door shares', () => { + // The integration half: a raw registerObject call (no Zod, no lint — + // the #3896 class of door) still gets the diagnostic. + const spy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const registry = new SchemaRegistry(); + registry.registerObject( + { name: 'fc_via_register', label: 'X', fields: { dead: { type: 'formula' } } } as any, + 'test-pkg', + ); + const hit = spy.mock.calls.find( + (c) => typeof c[0] === 'string' && (c[0] as string).includes('fc_via_register'), + ); + expect(hit).toBeDefined(); + expect(hit![0]).toContain('field/formula-without-expression'); + } finally { + spy.mockRestore(); + } + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 4d395b4ef5..04c0141690 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -3,7 +3,7 @@ import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary, resolveCrudAffordances, isTenancyDisabled, LEGACY_API_METHODS, AUDIT_PROVENANCE_FIELDS, type AuditProvenanceField } from '@objectstack/spec/data'; import { resolveMultiOrgEnabled, resolveSearchPinyinEnabled } from '@objectstack/types'; import { provisionSearchCompanion } from './search-companion.js'; -import { ObjectStackManifest, ManifestSchema, InstalledPackage, InstalledPackageSchema } from '@objectstack/spec/kernel'; +import { ObjectStackManifest, ManifestSchema, InstalledPackage, InstalledPackageSchema, checkFieldCompleteness } from '@objectstack/spec/kernel'; import { AppSchema } from '@objectstack/spec/ui'; import { applyProtection } from '@objectstack/spec/shared'; @@ -583,6 +583,66 @@ export function warnStrippedLegacyApiMethods( ); } +/** Objects already diagnosed for functional completeness (once per object). */ +const warnedFunctionalCompleteness = new Set(); + +/** + * [ADR-0078 Phase 4] Registration-time functional-completeness diagnostic. + * + * The author-time gate (`@objectstack/lint`'s `validate-functional-completeness`) + * only protects metadata that passes through `os build` / `validate` / `lint`. + * The registry is the one choke point EVERY door goes through — declared + * stacks, plugin-provided objects, `extend` contributions, `saveMetaItem`, raw + * `registerObject` calls — including the doors that skip Zod and lint entirely. + * Two shipped instances prove those doors are real, not hypothetical: #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). + * + * Same shared predicate as the author-time gate (`checkFieldCompleteness` in + * `@objectstack/spec/kernel`), so the rule ids in this warning are the SAME ids + * the lint reports — an operator or an AI reading the boot log can grep the id + * straight into the docs and the suppression story. Judgement lives only in the + * predicate; if a rule seems wrong, fix it there, never here. + * + * WARN, never throw — deliberately, and not as a soft default: an inert field + * must not kill a boot that thousands of healthy objects share (ADR-0078 §1 + * maps error-severity to "the INSTANCE is dead", not "the system is dead"). + * The author-time gate is where errors block; the registry's job is to make + * sure the silence never survives to runtime unobserved. + * + * Emitted once per object name with every finding aggregated into that one + * line (not per request, not per finding — the hot path stays free and a + * 3-dead-field object is one greppable line, not three). + */ +export function warnFunctionalCompleteness( + schema: ServiceObject, + opts?: { warn?: (msg: string) => void }, +): void { + const fields = (schema as { fields?: Record }).fields; + if (!fields || typeof fields !== 'object') return; + const name = String((schema as { name?: unknown }).name ?? ''); + if (warnedFunctionalCompleteness.has(name)) return; + + const findings: string[] = []; + for (const [fieldName, def] of Object.entries(fields)) { + for (const f of checkFieldCompleteness(def)) { + findings.push(`${fieldName}: [${f.severity}] ${f.rule} — add \`${f.fix}\``); + } + } + if (findings.length === 0) return; + warnedFunctionalCompleteness.add(name); + + const warn = opts?.warn ?? ((msg: string) => console.warn(msg)); + warn( + `[Registry] Object "${name}" registered with ${findings.length} functionally-incomplete ` + + `field(s) — Zod-valid but runtime-DEAD: the consumer silently skips each one, so it reads ` + + `0/null/never-resolves while every authoring surface reports success (ADR-0078). ` + + findings.join(' · ') + + ` — \`os lint\` reports the same rule ids with full context.`, + ); +} + /** * Platform namespaces that multiple packages may legitimately share, so the * install-time namespace-uniqueness gate (ADR-0048 Phase 1) must never fire on @@ -878,6 +938,13 @@ export class SchemaRegistry { // reconcile so we diagnose what actually ships. warnStrippedLegacyApiMethods(schema); + // [ADR-0078 Phase 4] One-shot per-object functional-completeness + // diagnostic — the registry is the choke point every metadata door goes + // through, including the ones that skip Zod and lint (#3896, raw + // registerObject). Same shared predicate and rule ids as `os lint`; + // warn-never-throw (an inert FIELD must not kill the boot). + warnFunctionalCompleteness(schema); + // [ADR-0079] Object-materialization seam — DESIGNATE-ONLY primary-title // provisioning. Runs AFTER `applySystemFields` (so any designated field // co-exists with the injected system columns) and ONLY for owned objects diff --git a/packages/plugins/plugin-webhooks/src/auto-enqueuer.test.ts b/packages/plugins/plugin-webhooks/src/auto-enqueuer.test.ts index b0d051620f..a9a0815ed1 100644 --- a/packages/plugins/plugin-webhooks/src/auto-enqueuer.test.ts +++ b/packages/plugins/plugin-webhooks/src/auto-enqueuer.test.ts @@ -329,6 +329,33 @@ describe('AutoEnqueuer', () => { await ae.stop(); }); + it('says OUT LOUD that a zero-trigger webhook will never fire (ADR-0078 Phase 4)', async () => { + // The skip used to be silent, under a comment blessing it as "a + // manual-only webhook" — a mode #3196 removed (no manual fire path + // exists). A zero-trigger ACTIVE row is a dead subscription that looks + // armed in Setup, so the skip now warns with the same rule id the + // author-time gate reports (`webhook/without-triggers`). Inactive rows + // never reach parseRow (the cache query filters `active: true`), so a + // deliberately-disabled webhook stays warning-free. + const engine = new FakeEngine({ sys_webhook: [webhook({ triggers: '' })] }); + const realtime = new FakeRealtime(); + const { enqueue, calls } = makeRecorder(); + const warn = vi.fn(); + const ae = new AutoEnqueuer(engine, realtime, enqueue, { refreshIntervalMs: 0, logger: { warn } }); + await ae.start(); + + await realtime.publish(event('created', 'contact', { id: 'c-1' })); + await flush(); + + expect(calls).toHaveLength(0); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('webhook/without-triggers'), + expect.objectContaining({ id: 'wh-1' }), + ); + expect(String(warn.mock.calls[0][0])).toContain('NEVER fire'); + await ae.stop(); + }); + it('self-heals the cache when sys_webhook changes', async () => { const engine = new FakeEngine({ sys_webhook: [] }); const realtime = new FakeRealtime(); diff --git a/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts b/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts index e3e64813bc..2161a4b8db 100644 --- a/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts +++ b/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts @@ -244,8 +244,25 @@ export class AutoEnqueuer { normalized.filter((t) => DISPATCHABLE_WEBHOOK_TRIGGERS.has(t)) as Array<'create' | 'update' | 'delete'>, ); if (triggers.size === 0) { - // No dispatchable triggers (or a manual-only webhook with none) — - // skip auto-enqueue. + // [ADR-0078 Phase 4] No dispatchable triggers — the webhook can + // never fire on ANY path, so say so instead of skipping silently. + // This comment used to read "(or a manual-only webhook with + // none)", but that mode does not exist: the `api` trigger was + // REMOVED (#3196, `webhook.zod.ts`) precisely because there is no + // manual fire path — the only webhook HTTP surface re-queues + // already-failed deliveries. So a zero-trigger row is not an off + // switch (that is `active`), it is a dead subscription that looks + // armed in Setup. Same rule id as the author-time gate + // (`webhook/without-triggers`) so the boot log greps into the + // same docs. Only active rows reach parseRow, so a deliberately + // disabled webhook stays warning-free. + this.logger.warn?.( + `[webhook-auto-enqueuer] webhook '${(row.name as string) ?? row.id}' has no dispatchable ` + + `triggers — it will NEVER fire (rule webhook/without-triggers): there is no manual fire ` + + `path (#3196), so this row is dead while looking armed in Setup. Declare ` + + `triggers: ['create'|'update'|'delete'], or set it inactive if it should be off.`, + { id: row.id }, + ); return null; }