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
44 changes: 44 additions & 0 deletions .changeset/adr-0078-phase3-webhook-triggers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
'@objectstack/spec': minor
'@objectstack/lint': minor
---

ADR-0078 Phase 3: a webhook with no `triggers` now fails at author time — and the Tier-B candidate list is corrected to what verification actually supports.

**The rule.** `webhook/without-triggers`, error severity, in the shared `@objectstack/spec/kernel` predicate alongside the Phase 1 rules, walked by `@objectstack/lint`'s `validate-functional-completeness` over `stack.webhooks` in both collection spellings. A webhook that declares no trigger materializes into `sys_webhook`, renders in Setup looking armed, and delivers nothing.

**Why it needed two sources, and why the first one argued against it.** The runtime skip site reads:

```
if (triggers.size === 0) {
// No dispatchable triggers (or a manual-only webhook with none) —
// skip auto-enqueue.
return null;
```

That parenthetical *blesses* the empty case as a deliberate mode — structurally identical to the `multiselect`-without-options NON-rule, where `record-validator.ts`'s `// free-form (tags without options)` is exactly why we do not flag it. On that evidence alone this candidate stays unenforced.

The mode it names does not exist. `webhook.zod.ts`'s #3196 note records that the `api` (manual/programmatic fire) trigger was *removed* because "no manual fire path exists — the only webhook HTTP surface re-queues already-failed deliveries". There is no way to fire a webhook the auto-enqueuer dropped. Inert on every path, so: `error`.

> **The generalization, now written into the module and pinned by a test:** a runtime comment records what its author believed, and beliefs go stale when a sibling feature is deleted. A blessing has to be corroborated by something showing the blessed mode is still *reachable* — otherwise it is a comment about a mode that no longer exists. The test asserts the finding carries both citations, so nobody demotes this rule on the strength of the comment alone.

`triggers: []` is flagged identically to an omitted `triggers`. Unlike an action's `locations: []` — the documented headless spelling — an empty array here carries no "I meant it" signal, because turning a webhook off has its own key (`isActive`). The repo's one real webhook (`showcase_task_changed`) confirms it: shipped inactive via `isActive: false`, with a full trigger list.

**The corrected Tier-B disposition.** Phase 3 was scoped from the 2026-06 audit's Tier-A/B catalog. Verifying each candidate before writing it — the discipline that caught four false prescriptions in #4001 — found most of the list already closed or misfiled:

| candidate | disposition |
|---|---|
| A2 action without `locations` | **already shipped** — `validate-action-locations.ts`, which already exempts the documented `locations: []` |
| B approval empty/unresolvable approvers | **already shipped** — `validate-approval-approvers.ts` |
| B select/multiselect without options | shipped in Phase 1 |
| B write-side referential integrity | **not an authoring-lint item** — a runtime gap; no metadata omission to detect |
| B `unique:true` no-op on memory driver | **not an authoring-lint item** — a driver gap |
| B composite/repeater sub-field constraints | **not an authoring-lint item** — a runtime gap |
| B nav targets of type page/report/url/component/action | **genuine gap, different module** — the key is present but dangling, which is reference resolvability (ADR-0072), not completeness (ADR-0078) |
| B dataset with zero measures | **unverified — not shipped.** No runtime consumer in this repo; the dataset compiler lives elsewhere |
| B webhook without triggers | ✅ **this change** |
| B schedule trigger with invalid cron | **unverified — not shipped.** `normalizeSchedule` accepts any non-empty string, but the scheduler's behaviour on an invalid one was not traced |

Two candidates are deliberately left unshipped rather than written on the audit's stated confidence, and one is left for the module that actually owns it. The audit's own lesson stands: it produces *candidates*, not confirmed bugs — the scariest one collapsed on a three-file read.

Tracked in #4544.
16 changes: 16 additions & 0 deletions packages/lint/src/validate-functional-completeness.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,20 @@ describe('validateFunctionalCompleteness — the walk', () => {
expect(findings.every((f) => f.severity === 'warning')).toBe(true);
});

it('walks webhooks in both spellings', () => {
expect(validateFunctionalCompleteness({
webhooks: [{ name: 'notify', url: 'https://x' }],
})[0]).toMatchObject({
rule: 'webhook/without-triggers',
severity: 'error',
where: 'webhook "notify"',
path: 'webhooks[0].triggers',
});
expect(validateFunctionalCompleteness({
webhooks: { notify: { url: 'https://x' } },
})[0]).toMatchObject({ where: 'webhook "notify"', path: 'webhooks.notify.triggers' });
});

it('is silent on a complete stack', () => {
expect(validateFunctionalCompleteness({
objects: [{
Expand All @@ -97,6 +111,7 @@ describe('validateFunctionalCompleteness — the walk', () => {
],
}],
views: [{ object: 'order', list: { type: 'grid' } }],
webhooks: [{ name: 'notify', url: 'https://x', triggers: ['create'] }],
})).toEqual([]);
});

Expand All @@ -107,6 +122,7 @@ describe('validateFunctionalCompleteness — the walk', () => {
{ objects: [{ name: 'o', fields: 'nope' }] },
{ views: [{ list: null }] },
{ views: 'nope' },
{ webhooks: 'nope' }, { webhooks: [null, 7] },
]) {
expect(() => validateFunctionalCompleteness(junk)).not.toThrow();
}
Expand Down
15 changes: 15 additions & 0 deletions packages/lint/src/validate-functional-completeness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import {
checkFieldCompleteness,
checkViewCompleteness,
checkWebhookCompleteness,
type CompletenessFinding,
} from '@objectstack/spec/kernel';

Expand Down Expand Up @@ -117,5 +118,19 @@ export function validateFunctionalCompleteness(stack: unknown): FunctionalComple
}
}

// ── Webhooks: stack.webhooks[] ─────────────────────────────────────────
// [ADR-0078 Phase 3] The one Tier-B candidate that survived its verification
// pass. A webhook materializes into `sys_webhook` and looks armed in Setup
// whether or not it declares a trigger, so the omission is invisible on every
// surface an author can see.
for (const hook of entriesOf(stack.webhooks)) {
push(
out,
checkWebhookCompleteness(hook.def),
`webhook "${hook.name}"`,
`webhooks${hook.key}`,
);
}

return out;
}
2 changes: 2 additions & 0 deletions packages/spec/api-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -1896,8 +1896,10 @@
"VersionConstraint (type)",
"VersionConstraintSchema (const)",
"VulnerabilitySeverity (type)",
"WEBHOOK_WITHOUT_TRIGGERS (const)",
"checkFieldCompleteness (function)",
"checkViewCompleteness (function)",
"checkWebhookCompleteness (function)",
"classifyRequiredCapability (function)",
"deriveNamespaceFromPackageId (function)",
"evaluateLockForDelete (function)",
Expand Down
50 changes: 49 additions & 1 deletion packages/spec/src/kernel/functional-completeness.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ import { describe, expect, it } from 'vitest';
import {
checkFieldCompleteness,
checkViewCompleteness,
checkWebhookCompleteness,
FUNCTIONAL_COMPLETENESS_RULES,
FIELD_SUMMARY_WITHOUT_OPERATIONS,
FIELD_FORMULA_WITHOUT_EXPRESSION,
FIELD_RELATIONSHIP_WITHOUT_REFERENCE,
FIELD_CHOICE_WITHOUT_OPTIONS,
VIEW_LAYOUT_WITHOUT_BINDING,
WEBHOOK_WITHOUT_TRIGGERS,
} from './functional-completeness';

const only = (findings: ReturnType<typeof checkFieldCompleteness>) => {
Expand Down Expand Up @@ -135,6 +137,50 @@ describe('checkViewCompleteness — layout bindings', () => {
});
});

describe('checkWebhookCompleteness — the rule the runtime comment argued against', () => {
it('flags a webhook with no `triggers` as an ERROR', () => {
const f = only(checkWebhookCompleteness({ name: 'notify_slack', url: 'https://x' }) as never);
expect(f.rule).toBe(WEBHOOK_WITHOUT_TRIGGERS);
expect(f.severity).toBe('error');
});

it('flags `triggers: []` the same — an empty array is not an off switch here', () => {
// Contrast with an action's `locations: []`, which IS the documented
// headless spelling. A webhook's off switch is `isActive: false`, so an
// empty trigger list carries no "I meant it" signal — it is the same dead
// shape written out longhand.
const f = only(checkWebhookCompleteness({ triggers: [] }) as never);
expect(f.severity).toBe('error');
});

it('is silent once a trigger is declared', () => {
expect(checkWebhookCompleteness({ triggers: ['create'] })).toEqual([]);
expect(checkWebhookCompleteness({ triggers: ['create', 'update', 'delete'] })).toEqual([]);
});

it('carries BOTH sources, because either one alone gets this wrong', () => {
// The skip site's own comment says "or a manual-only webhook with none",
// which reads as a runtime blessing — the exact shape that makes
// `multiselect` a NON-rule. What defeats it is webhook.zod.ts's #3196 note
// that no manual fire path exists, so the blessed mode is unreachable.
// If someone later demotes or deletes this rule on the strength of that
// comment alone, this assertion is where the missing half is stated.
const [f] = checkWebhookCompleteness({});
expect(f.message).toContain('auto-enqueuer.ts');
expect(f.message).toContain('no manual fire path exists');
expect(f.message).toContain('isActive');
});

it('never throws on junk', () => {
for (const junk of [undefined, null, 42, 'x', [], { triggers: 'create' }, { triggers: 7 }]) {
expect(() => checkWebhookCompleteness(junk)).not.toThrow();
}
// A non-array `triggers` is not a declared trigger list — it is the dead
// shape wearing the wrong type, so it must not slip through as "declared".
expect(checkWebhookCompleteness({ triggers: 'create' })).toHaveLength(1);
});
});

describe('registry hygiene', () => {
it('pins the rule-id list — ids are API for suppressions and dashboards', () => {
expect([...FUNCTIONAL_COMPLETENESS_RULES].sort()).toEqual([
Expand All @@ -143,6 +189,7 @@ describe('registry hygiene', () => {
'field/relationship-without-reference',
'field/summary-without-operations',
'view/layout-without-binding',
'webhook/without-triggers',
]);
});

Expand All @@ -154,8 +201,9 @@ describe('registry hygiene', () => {
...checkFieldCompleteness({ type: 'select' }),
...checkFieldCompleteness({ type: 'checkboxes' }),
...checkViewCompleteness({ type: 'kanban' }),
...checkWebhookCompleteness({ url: 'https://x' }),
];
expect(all).toHaveLength(6);
expect(all).toHaveLength(7);
for (const f of all) {
expect(f.fix.length).toBeGreaterThan(8);
expect(f.message.length).toBeGreaterThan(60);
Expand Down
64 changes: 64 additions & 0 deletions packages/spec/src/kernel/functional-completeness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@
* - `checkboxes` w/o `options` sits between the two: it shares the multi
* branch's free-form validator behaviour, but a checkbox group with zero
* boxes is almost certainly an omission — so it is a WARNING, not an error.
* - `webhook` w/o `triggers` → `auto-enqueuer.ts` `if (triggers.size === 0) …
* return null`. Note this one needed a SECOND source: that skip site's own
* comment blesses the empty case as "a manual-only webhook", which reads
* exactly like the `multiselect` exemption above — but `webhook.zod.ts`
* (#3196) records that no manual fire path exists, so the blessed mode is
* unreachable. See {@link checkWebhookCompleteness}. A runtime comment states
* what its author believed; a removed sibling feature can make it stale.
*
* Severity follows ADR-0078 decision 1: `error` when the instance is fully
* inert, `warning` when it degrades to something that partially works.
Expand All @@ -70,6 +77,7 @@ export const FIELD_FORMULA_WITHOUT_EXPRESSION = 'field/formula-without-expressio
export const FIELD_RELATIONSHIP_WITHOUT_REFERENCE = 'field/relationship-without-reference';
export const FIELD_CHOICE_WITHOUT_OPTIONS = 'field/choice-without-options';
export const VIEW_LAYOUT_WITHOUT_BINDING = 'view/layout-without-binding';
export const WEBHOOK_WITHOUT_TRIGGERS = 'webhook/without-triggers';

/** Every rule id this module can emit — pinned by tests so ids cannot drift. */
export const FUNCTIONAL_COMPLETENESS_RULES = [
Expand All @@ -78,6 +86,7 @@ export const FUNCTIONAL_COMPLETENESS_RULES = [
FIELD_RELATIONSHIP_WITHOUT_REFERENCE,
FIELD_CHOICE_WITHOUT_OPTIONS,
VIEW_LAYOUT_WITHOUT_BINDING,
WEBHOOK_WITHOUT_TRIGGERS,
] as const;

type AnyRec = Record<string, unknown>;
Expand Down Expand Up @@ -222,3 +231,58 @@ export function checkViewCompleteness(view: unknown): CompletenessFinding[] {
: "gantt: { startDateField: '<date_field>', endDateField: '<date_field>', titleField: '<text_field>' }",
}];
}

/**
* Completeness of a single webhook definition.
*
* ## Why this one needed TWO sources, and why one of them alone was misleading
*
* The auto-enqueuer's own comment reads, at the skip site:
*
* ```
* if (triggers.size === 0) {
* // No dispatchable triggers (or a manual-only webhook with none) —
* // skip auto-enqueue.
* return null;
* ```
*
* Read alone, that parenthetical *blesses* the empty case as a deliberate mode
* — exactly the shape that makes `multiselect` without options a NON-rule
* above. Stopping there would have left this candidate unenforced.
*
* But the mode it names does not exist. `webhook.zod.ts`'s #3196 note records
* that `api` (manual/programmatic fire) was REMOVED as a trigger value
* precisely because "no manual fire path exists (the only webhook HTTP surface
* re-queues already-failed deliveries)". So there is no way to fire a webhook
* that the auto-enqueuer has dropped: it is inert on every path, not
* manual-only. Hence `error`, not a NON-rule.
*
* The lesson generalizes past this rule: a runtime comment describes what its
* author believed, and beliefs go stale when a sibling feature is removed. The
* blessing has to be corroborated by something that says the blessed mode is
* REACHABLE — otherwise it is a comment about a mode that no longer exists.
*
* `triggers: []` is flagged the same as an omitted `triggers`: unlike an
* action's `locations: []` (the documented headless spelling), an empty array
* here is not an "I meant it" marker — turning a webhook OFF has its own key
* (`isActive` → the row's `active`), so `[]` is simply the same dead shape
* spelled out.
*/
export function checkWebhookCompleteness(webhook: unknown): CompletenessFinding[] {
if (!isRec(webhook)) return [];
if (hasEntries(webhook.triggers)) return [];
return [{
rule: WEBHOOK_WITHOUT_TRIGGERS,
severity: 'error',
path: 'triggers',
message:
'A webhook with no `triggers` never fires on any path. The auto-enqueuer drops it while '
+ 'building its subscription cache (`auto-enqueuer.ts` — `if (triggers.size === 0) … return '
+ 'null`), and there is no manual fire path to reach it either: `webhook.zod.ts` (#3196) '
+ 'records that the `api` trigger was removed because "no manual fire path exists — the only '
+ 'webhook HTTP surface re-queues already-failed deliveries". The webhook materializes into '
+ '`sys_webhook`, looks armed in Setup, and delivers nothing. To disable a webhook use '
+ '`isActive: false`; an empty `triggers` is not an off switch, just a dead one.',
fix: "triggers: ['create', 'update']",
}];
}
Loading