Skip to content

Commit e6e9379

Browse files
os-zhuangclaude
andauthored
feat(lint): webhook without triggers fails at author time (ADR-0078 Phase 3) (#4565)
A webhook that declares no `triggers` never fires on any path. It still materializes into `sys_webhook` and still renders in Setup looking armed, so the omission is invisible on every surface an author can see — the ADR-0078 shape. ## The first source argued AGAINST this rule 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. It is structurally identical to the `multiselect`-without-options NON-rule, where `record-validator.ts`'s `// free-form (tags without options)` is precisely why we do NOT flag it. On this evidence alone the 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". Nothing can fire a webhook the auto-enqueuer dropped. Inert on every path, so: error, not a NON-rule. The generalization is now in the module doc 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 needs corroboration that the blessed mode is still reachable, or it is a comment about a mode that no longer exists. The test asserts the finding carries both citations so nobody demotes the rule on 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 a webhook's off switch is `isActive`. The repo's one real webhook confirms the premise — `showcase_task_changed` ships inactive via `isActive: false` with a full trigger list, and is clean. ## Phase 3's scope was mostly already closed Verifying each Tier-B candidate before writing it — the discipline that caught four false prescriptions in #4001 — found most of the audit's list already handled or misfiled: A2 action without `locations` already shipped (validate-action-locations, which already exempts `locations: []`) approval empty approvers already shipped (validate-approval-approvers) select/multiselect options shipped in Phase 1 write-side FK integrity not an authoring-lint item (runtime gap) `unique` on memory driver not an authoring-lint item (driver gap) composite sub-field constraints not an authoring-lint item (runtime gap) nav page/report/url/component genuine gap, but reference resolvability (ADR-0072), not completeness — wrong module dataset with zero measures UNVERIFIED, not shipped: no runtime consumer in this repo schedule trigger invalid cron UNVERIFIED, not shipped: normalizeSchedule takes any non-empty string, scheduler behaviour not traced webhook without triggers this change Two candidates are left unshipped rather than written on the audit's stated confidence, and one is left to the module that owns it. Verification: 24 predicate tests (19 -> 24) and 8 walk tests (7 -> 8); the one real webhook in the repo produces no finding; full suite 132/132. Tracked in #4544. Claude-Session: https://claude.ai/code/session_01WnqGjQFQMqd5k81LYV8SCY Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3063352 commit e6e9379

6 files changed

Lines changed: 190 additions & 1 deletion
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/lint': minor
4+
---
5+
6+
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.
7+
8+
**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.
9+
10+
**Why it needed two sources, and why the first one argued against it.** The runtime skip site reads:
11+
12+
```
13+
if (triggers.size === 0) {
14+
// No dispatchable triggers (or a manual-only webhook with none) —
15+
// skip auto-enqueue.
16+
return null;
17+
```
18+
19+
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.
20+
21+
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`.
22+
23+
> **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.
24+
25+
`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.
26+
27+
**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:
28+
29+
| candidate | disposition |
30+
|---|---|
31+
| A2 action without `locations` | **already shipped**`validate-action-locations.ts`, which already exempts the documented `locations: []` |
32+
| B approval empty/unresolvable approvers | **already shipped**`validate-approval-approvers.ts` |
33+
| B select/multiselect without options | shipped in Phase 1 |
34+
| B write-side referential integrity | **not an authoring-lint item** — a runtime gap; no metadata omission to detect |
35+
| B `unique:true` no-op on memory driver | **not an authoring-lint item** — a driver gap |
36+
| B composite/repeater sub-field constraints | **not an authoring-lint item** — a runtime gap |
37+
| 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) |
38+
| B dataset with zero measures | **unverified — not shipped.** No runtime consumer in this repo; the dataset compiler lives elsewhere |
39+
| B webhook without triggers |**this change** |
40+
| 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 |
41+
42+
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.
43+
44+
Tracked in #4544.

packages/lint/src/validate-functional-completeness.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,20 @@ describe('validateFunctionalCompleteness — the walk', () => {
8585
expect(findings.every((f) => f.severity === 'warning')).toBe(true);
8686
});
8787

88+
it('walks webhooks in both spellings', () => {
89+
expect(validateFunctionalCompleteness({
90+
webhooks: [{ name: 'notify', url: 'https://x' }],
91+
})[0]).toMatchObject({
92+
rule: 'webhook/without-triggers',
93+
severity: 'error',
94+
where: 'webhook "notify"',
95+
path: 'webhooks[0].triggers',
96+
});
97+
expect(validateFunctionalCompleteness({
98+
webhooks: { notify: { url: 'https://x' } },
99+
})[0]).toMatchObject({ where: 'webhook "notify"', path: 'webhooks.notify.triggers' });
100+
});
101+
88102
it('is silent on a complete stack', () => {
89103
expect(validateFunctionalCompleteness({
90104
objects: [{
@@ -97,6 +111,7 @@ describe('validateFunctionalCompleteness — the walk', () => {
97111
],
98112
}],
99113
views: [{ object: 'order', list: { type: 'grid' } }],
114+
webhooks: [{ name: 'notify', url: 'https://x', triggers: ['create'] }],
100115
})).toEqual([]);
101116
});
102117

@@ -107,6 +122,7 @@ describe('validateFunctionalCompleteness — the walk', () => {
107122
{ objects: [{ name: 'o', fields: 'nope' }] },
108123
{ views: [{ list: null }] },
109124
{ views: 'nope' },
125+
{ webhooks: 'nope' }, { webhooks: [null, 7] },
110126
]) {
111127
expect(() => validateFunctionalCompleteness(junk)).not.toThrow();
112128
}

packages/lint/src/validate-functional-completeness.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import {
2626
checkFieldCompleteness,
2727
checkViewCompleteness,
28+
checkWebhookCompleteness,
2829
type CompletenessFinding,
2930
} from '@objectstack/spec/kernel';
3031

@@ -117,5 +118,19 @@ export function validateFunctionalCompleteness(stack: unknown): FunctionalComple
117118
}
118119
}
119120

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

packages/spec/api-surface.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1896,8 +1896,10 @@
18961896
"VersionConstraint (type)",
18971897
"VersionConstraintSchema (const)",
18981898
"VulnerabilitySeverity (type)",
1899+
"WEBHOOK_WITHOUT_TRIGGERS (const)",
18991900
"checkFieldCompleteness (function)",
19001901
"checkViewCompleteness (function)",
1902+
"checkWebhookCompleteness (function)",
19011903
"classifyRequiredCapability (function)",
19021904
"deriveNamespaceFromPackageId (function)",
19031905
"evaluateLockForDelete (function)",

packages/spec/src/kernel/functional-completeness.test.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@ import { describe, expect, it } from 'vitest';
2020
import {
2121
checkFieldCompleteness,
2222
checkViewCompleteness,
23+
checkWebhookCompleteness,
2324
FUNCTIONAL_COMPLETENESS_RULES,
2425
FIELD_SUMMARY_WITHOUT_OPERATIONS,
2526
FIELD_FORMULA_WITHOUT_EXPRESSION,
2627
FIELD_RELATIONSHIP_WITHOUT_REFERENCE,
2728
FIELD_CHOICE_WITHOUT_OPTIONS,
2829
VIEW_LAYOUT_WITHOUT_BINDING,
30+
WEBHOOK_WITHOUT_TRIGGERS,
2931
} from './functional-completeness';
3032

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

140+
describe('checkWebhookCompleteness — the rule the runtime comment argued against', () => {
141+
it('flags a webhook with no `triggers` as an ERROR', () => {
142+
const f = only(checkWebhookCompleteness({ name: 'notify_slack', url: 'https://x' }) as never);
143+
expect(f.rule).toBe(WEBHOOK_WITHOUT_TRIGGERS);
144+
expect(f.severity).toBe('error');
145+
});
146+
147+
it('flags `triggers: []` the same — an empty array is not an off switch here', () => {
148+
// Contrast with an action's `locations: []`, which IS the documented
149+
// headless spelling. A webhook's off switch is `isActive: false`, so an
150+
// empty trigger list carries no "I meant it" signal — it is the same dead
151+
// shape written out longhand.
152+
const f = only(checkWebhookCompleteness({ triggers: [] }) as never);
153+
expect(f.severity).toBe('error');
154+
});
155+
156+
it('is silent once a trigger is declared', () => {
157+
expect(checkWebhookCompleteness({ triggers: ['create'] })).toEqual([]);
158+
expect(checkWebhookCompleteness({ triggers: ['create', 'update', 'delete'] })).toEqual([]);
159+
});
160+
161+
it('carries BOTH sources, because either one alone gets this wrong', () => {
162+
// The skip site's own comment says "or a manual-only webhook with none",
163+
// which reads as a runtime blessing — the exact shape that makes
164+
// `multiselect` a NON-rule. What defeats it is webhook.zod.ts's #3196 note
165+
// that no manual fire path exists, so the blessed mode is unreachable.
166+
// If someone later demotes or deletes this rule on the strength of that
167+
// comment alone, this assertion is where the missing half is stated.
168+
const [f] = checkWebhookCompleteness({});
169+
expect(f.message).toContain('auto-enqueuer.ts');
170+
expect(f.message).toContain('no manual fire path exists');
171+
expect(f.message).toContain('isActive');
172+
});
173+
174+
it('never throws on junk', () => {
175+
for (const junk of [undefined, null, 42, 'x', [], { triggers: 'create' }, { triggers: 7 }]) {
176+
expect(() => checkWebhookCompleteness(junk)).not.toThrow();
177+
}
178+
// A non-array `triggers` is not a declared trigger list — it is the dead
179+
// shape wearing the wrong type, so it must not slip through as "declared".
180+
expect(checkWebhookCompleteness({ triggers: 'create' })).toHaveLength(1);
181+
});
182+
});
183+
138184
describe('registry hygiene', () => {
139185
it('pins the rule-id list — ids are API for suppressions and dashboards', () => {
140186
expect([...FUNCTIONAL_COMPLETENESS_RULES].sort()).toEqual([
@@ -143,6 +189,7 @@ describe('registry hygiene', () => {
143189
'field/relationship-without-reference',
144190
'field/summary-without-operations',
145191
'view/layout-without-binding',
192+
'webhook/without-triggers',
146193
]);
147194
});
148195

@@ -154,8 +201,9 @@ describe('registry hygiene', () => {
154201
...checkFieldCompleteness({ type: 'select' }),
155202
...checkFieldCompleteness({ type: 'checkboxes' }),
156203
...checkViewCompleteness({ type: 'kanban' }),
204+
...checkWebhookCompleteness({ url: 'https://x' }),
157205
];
158-
expect(all).toHaveLength(6);
206+
expect(all).toHaveLength(7);
159207
for (const f of all) {
160208
expect(f.fix.length).toBeGreaterThan(8);
161209
expect(f.message.length).toBeGreaterThan(60);

packages/spec/src/kernel/functional-completeness.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@
4646
* - `checkboxes` w/o `options` sits between the two: it shares the multi
4747
* branch's free-form validator behaviour, but a checkbox group with zero
4848
* boxes is almost certainly an omission — so it is a WARNING, not an error.
49+
* - `webhook` w/o `triggers` → `auto-enqueuer.ts` `if (triggers.size === 0) …
50+
* return null`. Note this one needed a SECOND source: that skip site's own
51+
* comment blesses the empty case as "a manual-only webhook", which reads
52+
* exactly like the `multiselect` exemption above — but `webhook.zod.ts`
53+
* (#3196) records that no manual fire path exists, so the blessed mode is
54+
* unreachable. See {@link checkWebhookCompleteness}. A runtime comment states
55+
* what its author believed; a removed sibling feature can make it stale.
4956
*
5057
* Severity follows ADR-0078 decision 1: `error` when the instance is fully
5158
* inert, `warning` when it degrades to something that partially works.
@@ -70,6 +77,7 @@ export const FIELD_FORMULA_WITHOUT_EXPRESSION = 'field/formula-without-expressio
7077
export const FIELD_RELATIONSHIP_WITHOUT_REFERENCE = 'field/relationship-without-reference';
7178
export const FIELD_CHOICE_WITHOUT_OPTIONS = 'field/choice-without-options';
7279
export const VIEW_LAYOUT_WITHOUT_BINDING = 'view/layout-without-binding';
80+
export const WEBHOOK_WITHOUT_TRIGGERS = 'webhook/without-triggers';
7381

7482
/** Every rule id this module can emit — pinned by tests so ids cannot drift. */
7583
export const FUNCTIONAL_COMPLETENESS_RULES = [
@@ -78,6 +86,7 @@ export const FUNCTIONAL_COMPLETENESS_RULES = [
7886
FIELD_RELATIONSHIP_WITHOUT_REFERENCE,
7987
FIELD_CHOICE_WITHOUT_OPTIONS,
8088
VIEW_LAYOUT_WITHOUT_BINDING,
89+
WEBHOOK_WITHOUT_TRIGGERS,
8190
] as const;
8291

8392
type AnyRec = Record<string, unknown>;
@@ -222,3 +231,58 @@ export function checkViewCompleteness(view: unknown): CompletenessFinding[] {
222231
: "gantt: { startDateField: '<date_field>', endDateField: '<date_field>', titleField: '<text_field>' }",
223232
}];
224233
}
234+
235+
/**
236+
* Completeness of a single webhook definition.
237+
*
238+
* ## Why this one needed TWO sources, and why one of them alone was misleading
239+
*
240+
* The auto-enqueuer's own comment reads, at the skip site:
241+
*
242+
* ```
243+
* if (triggers.size === 0) {
244+
* // No dispatchable triggers (or a manual-only webhook with none) —
245+
* // skip auto-enqueue.
246+
* return null;
247+
* ```
248+
*
249+
* Read alone, that parenthetical *blesses* the empty case as a deliberate mode
250+
* — exactly the shape that makes `multiselect` without options a NON-rule
251+
* above. Stopping there would have left this candidate unenforced.
252+
*
253+
* But the mode it names does not exist. `webhook.zod.ts`'s #3196 note records
254+
* that `api` (manual/programmatic fire) was REMOVED as a trigger value
255+
* precisely because "no manual fire path exists (the only webhook HTTP surface
256+
* re-queues already-failed deliveries)". So there is no way to fire a webhook
257+
* that the auto-enqueuer has dropped: it is inert on every path, not
258+
* manual-only. Hence `error`, not a NON-rule.
259+
*
260+
* The lesson generalizes past this rule: a runtime comment describes what its
261+
* author believed, and beliefs go stale when a sibling feature is removed. The
262+
* blessing has to be corroborated by something that says the blessed mode is
263+
* REACHABLE — otherwise it is a comment about a mode that no longer exists.
264+
*
265+
* `triggers: []` is flagged the same as an omitted `triggers`: unlike an
266+
* action's `locations: []` (the documented headless spelling), an empty array
267+
* here is not an "I meant it" marker — turning a webhook OFF has its own key
268+
* (`isActive` → the row's `active`), so `[]` is simply the same dead shape
269+
* spelled out.
270+
*/
271+
export function checkWebhookCompleteness(webhook: unknown): CompletenessFinding[] {
272+
if (!isRec(webhook)) return [];
273+
if (hasEntries(webhook.triggers)) return [];
274+
return [{
275+
rule: WEBHOOK_WITHOUT_TRIGGERS,
276+
severity: 'error',
277+
path: 'triggers',
278+
message:
279+
'A webhook with no `triggers` never fires on any path. The auto-enqueuer drops it while '
280+
+ 'building its subscription cache (`auto-enqueuer.ts` — `if (triggers.size === 0) … return '
281+
+ 'null`), and there is no manual fire path to reach it either: `webhook.zod.ts` (#3196) '
282+
+ 'records that the `api` trigger was removed because "no manual fire path exists — the only '
283+
+ 'webhook HTTP surface re-queues already-failed deliveries". The webhook materializes into '
284+
+ '`sys_webhook`, looks armed in Setup, and delivers nothing. To disable a webhook use '
285+
+ '`isActive: false`; an empty `triggers` is not an off switch, just a dead one.',
286+
fix: "triggers: ['create', 'update']",
287+
}];
288+
}

0 commit comments

Comments
 (0)