Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .changeset/adr-0078-phase4-runtime-warns.md
Original file line number Diff line number Diff line change
@@ -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.
101 changes: 100 additions & 1 deletion packages/objectql/src/registry.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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();
}
});
});
69 changes: 68 additions & 1 deletion packages/objectql/src/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -583,6 +583,66 @@ export function warnStrippedLegacyApiMethods(
);
}

/** Objects already diagnosed for functional completeness (once per object). */
const warnedFunctionalCompleteness = new Set<string>();

/**
* [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<string, unknown> }).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
Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions packages/plugins/plugin-webhooks/src/auto-enqueuer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
21 changes: 19 additions & 2 deletions packages/plugins/plugin-webhooks/src/auto-enqueuer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading