diff --git a/.changeset/unknown-key-strictness-automation-batch11.md b/.changeset/unknown-key-strictness-automation-batch11.md new file mode 100644 index 0000000000..f3a797c75b --- /dev/null +++ b/.changeset/unknown-key-strictness-automation-batch11.md @@ -0,0 +1,76 @@ +--- +'@objectstack/spec': major +--- + +Close nine authorable automation shapes against unknown keys (#4001 batch 11, ADR-0078) + +zod's default is `.strip`: a key a schema does not declare is silently discarded +and the parse still succeeds. On an authoring surface that is the worst failure +mode — the author (increasingly, an AI) gets a success envelope and ships +metadata that quietly ignores what they wrote. This batch closes the nine +remaining shapes in `automation/`'s main body. + +**BREAKING.** Each of these now raises a named, fixable error instead of dropping +the key. The rejection carries the surface, the offending key, and — where the +word is recognisable — the canonical spelling. + +**`flow.zod.ts` — the six NESTED blocks.** The four outer shapes (flow / node / +edge / variable) were closed earlier; their inner blocks were not, so the gate +rejected `nodee:` at node level while `connectorConfig: { connectorID }` — one +capital letter — parsed clean and dispatched the action against an undefined +connector id. Now strict: `FlowNode.connectorConfig`, `.position`, +`.inputSchema` (each parameter declaration), `.waitEventConfig`, +`.boundaryConfig`, and `Flow.errorHandling`. + +Renames the rejections offer, each one a real spelling of the same knob on a +neighbouring surface in this repo: + +| you wrote | write instead | where the other word comes from | +|---|---|---| +| `connectorConfig.params` / `parameters` / `arguments` / `payload` | `input` | script-node `config.inputs`, integration products | +| `waitEventConfig.event` / `signal` / `duration` / `delay` | `eventType` / `signalName` / `timerDuration` | — | +| `boundaryConfig.attachedToRef` / `cancelActivity` | `attachedToNodeId` / `interrupting` | BPMN 2.0's own attribute names | +| `errorHandling.backoffMs` | `retryDelayMs` | `shared/retry-policy.zod.ts` (#4661) | +| `errorHandling.initialDelayMs` / `maxDelayMs` | `retryDelayMs` / `maxRetryDelayMs` | connector `RetryConfig` | +| `errorHandling.retries` / `attempts` / `onError` | `maxRetries` / `strategy` | — | + +Two are prescriptions rather than renames, because a rename would be wrong: +`inputSchema`'s `optional` is the opposite polarity of `required` (write +`required: false`), and `errorHandling.maxAttempts` counts the first attempt +while `maxRetries` counts the ones after it (write `maxRetries: maxAttempts - 1`). + +**Deliberately still open**, both now pinned in code and in tests so a later +sweep stops rather than "finishing" the file: the flow node `config` slot +(ADR-0018 — the plugin node-type namespace, owned by each executor's +`configSchema`) and `FlowVersionHistorySchema` (emitted on publish, never +authored; the flow *inside* a history record is still gated by `FlowSchema`). + +**`time-relative-trigger.zod.ts`.** `config.timeRelative` sits under the open +node `config` slot, so this schema is the only key gate it has — and it is +`safeParse`d at BIND time, not only at authoring. `{ …valid, offsetDay: 7 }` +used to bind a sweep that ran daily with the author's narrowing discarded, and +reported itself configured; it now refuses to bind and says why. `field` → +`dateField`, `filters` → `filter`, `objectName` → `object`, `limit` → +`maxRecords`; `schedule` and `runAs` get pointed at the layer that owns them. + +**`flow-function.zod.ts`.** `{ handler, effect }` in `defineStack({ functions })`. +This binds at authoring only — the boot path reads entries with +`normalizeFlowFunctionEntry`, not a parse — which is exactly why it matters: +that reader takes two keys and ignores the rest by construction, so a misspelled +`effect` was dropped at the schema and then not looked for. The function still +registered, still ran, and its writes were still counted as none, keeping +#4354's broken-sweep query silent on the one run that needed it. + +**`webhook.zod.ts`.** `object_name` → `object` and `active` → `isActive` (the +`sys_webhook` column names, for anyone re-authoring from a row), `events` → +`triggers`, `endpoint` → `url`; the five props #3494 removed now reject with +their reason instead of vanishing. Strictness also rides `.extend()` onto the +connector `WebhookConfigSchema`. + +**`webhook` also gains the ADR-0010 protection envelope** (`protection`, plus +the loader-set `_lock` / `_lockReason` / `_lockSource` / `_provenance` / +`_packageId` / `_packageVersion` / `_lockDocsUrl`). This is not a separate +feature: both metadata load paths call `applyProtection` on every type, so a +package-loaded webhook already carried those keys when `plugin-webhooks` +re-parsed it at boot. Closing the shape without declaring them would have turned +every package-shipped webhook into a skipped subscription after a redeploy. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 28e90a4392..b6794e2549 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -144,7 +144,7 @@ Each node performs a specific action in the flow. | `position` | `{ x, y }` | optional | Visual position on canvas | | `timeoutMs` | `number` | optional | Per-node execution timeout | | `inputSchema` | `object` | optional | Declared input parameter types, for Studio form generation and runtime validation | -| `waitEventConfig` | `object` | optional | `wait`-node event descriptor (`eventType`, `timerDuration`, `signalName`, `timeoutMs`, `onTimeout`) | +| `waitEventConfig` | `object` | optional | `wait`-node event descriptor (`eventType`, `timerDuration`, `signalName`). `timeoutMs` / `onTimeout` were removed in 17 (#4158) — `wait` has no timeout; `timerDuration` accepts a bare number as milliseconds | | `boundaryConfig` | `object` | optional | BPMN boundary-event descriptor (interop) | diff --git a/content/docs/references/automation/webhook.mdx b/content/docs/references/automation/webhook.mdx index bd6e6514a5..bdd87562a3 100644 --- a/content/docs/references/automation/webhook.mdx +++ b/content/docs/references/automation/webhook.mdx @@ -92,6 +92,14 @@ const result = WebhookSchema.parse(data); | **secret** | `string` | optional | Signing secret for HMAC signature verification | | **isActive** | `boolean` | ✅ | Whether webhook is active | | **description** | `string` | optional | Webhook description | +| **protection** | `{ lock: Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>; reason: string; docsUrl?: string }` | optional | Package author protection block — lock policy for this webhook. | +| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | +| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | +| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). | +| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). | +| **_packageId** | `string` | optional | Owning package machine id. | +| **_packageVersion** | `string` | optional | Owning package version. | +| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. | --- diff --git a/content/docs/references/integration/connector.mdx b/content/docs/references/integration/connector.mdx index db1b39f3a0..dfeadc1cd3 100644 --- a/content/docs/references/integration/connector.mdx +++ b/content/docs/references/integration/connector.mdx @@ -487,6 +487,14 @@ Synchronization strategy | **secret** | `string` | optional | Signing secret for HMAC signature verification | | **isActive** | `boolean` | ✅ | Whether webhook is active | | **description** | `string` | optional | Webhook description | +| **protection** | `{ lock: Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>; reason: string; docsUrl?: string }` | optional | Package author protection block — lock policy for this webhook. | +| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | +| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | +| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). | +| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). | +| **_packageId** | `string` | optional | Owning package machine id. | +| **_packageVersion** | `string` | optional | Owning package version. | +| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. | | **events** | `Enum<'record.created' \| 'record.updated' \| 'record.deleted' \| 'sync.started' \| 'sync.completed' \| 'sync.failed' \| 'auth.expired' \| 'rate_limit.exceeded'>[]` | optional | Connector events to subscribe to (not yet enforced — no runtime dispatches these; see #3197) | | **signatureAlgorithm** | `Enum<'hmac_sha256' \| 'hmac_sha512' \| 'none'>` | ✅ | Webhook signature algorithm | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 5e8cf23ae2..929cf548cd 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -528,7 +528,7 @@ not verdicts). | File | Sites | Class | Note | |---|---|---|---| -| `flow.zod.ts` | 11 | authorable | **strict as of #4001** (4 schemas; `FlowVersionHistorySchema` is runtime — stays tolerant) | +| `flow.zod.ts` | 11 | authorable | **strict as of #4001** — the four outer authoring shapes at step 1, and **the six nested blocks at batch 11** (`FlowNode.connectorConfig` / `.position` / `.inputSchema` / `.waitEventConfig` / `.boundaryConfig`, `Flow.errorHandling`). The gap between those two dates is this campaign's own finding 17 inside its own file: closing the shells left the gate rejecting `nodee:` at node level while `connectorConfig: { connectorId, actionId, params: {…} }` parsed clean and the executor dispatched `input ?? {}` — a successful connector call carrying nothing. Worth recording precisely, because the obvious example is the wrong one: a slip on a REQUIRED key was always loud (it then reads as missing). What `.strip` swallowed here is the OPTIONAL half — the input map, the retry budget, `interrupting: false`, `required: true` — i.e. exactly the keys an author adds to CONSTRAIN behaviour, replaced by a permissive default without a word. Two things stay open and are now pinned in code with the reason, so a later sweep stops rather than "finishes" the file: the node `config` slot (ADR-0018 plugin namespace) and `FlowVersionHistorySchema` (the file's only WIRE shape — emitted on publish, never authored; its `definition` is `FlowSchema`, so the authored half inside a history record is gated anyway) | | `etl.zod.ts` | 10 | authorable (p) | authored pipelines — **candidate**. **−12 at #4738**: `sync.zod.ts` (the L1 "Simple Sync" file — `DataSyncConfig`, its `ConflictResolution` enum and satellites, formerly this row's co-candidate) was deleted whole rather than hardened: three-repo zero importers, no parse site, defs unreachable from the metadata-type roots (#4650 gate), so there was no author for strictness to protect (#4535 C13+C15). The integration-side `ConflictResolution` → `ConnectorConflictResolution` rename in the same change is name-only and moves no sites | | `execution.zod.ts` | 13 | wire | run-state envelopes — never strict. +5 at #4354 (the run-summary family: step metrics / skip reason / per-node / per-gate / the summary itself) — engine-emitted telemetry read by the Console and by operator queries, nobody authors them, so the `wire` verdict covers them unchanged | | `state-machine.zod.ts` | 6 | authorable | **strict as of #4001 批 10** — all six sites (`ActionRef` / `GuardRef` / `Transition` / `StateNode` + `.meta` / `StateMachine`). **The `(p)` was NOT a formality here.** ADR-0020 retired this XState shape as a *record-lifecycle* declaration — the top-level `workflow` metadata type and `object.stateMachines` are both gone, and a record's transitions live on the `state_machine` VALIDATION RULE instead — so had those been the only doors this file would be DEAD surface, and the correct action would have been to fix its class, not close it. One authoring door survives: `ai/agent.zod.ts`'s `lifecycle` is `StateMachineSchema`, and `agent` is a registered type, so `defineStack({ agents })` / meta REST / the Studio agent form all reach here through `AgentSchema.parse()`. Verified by parse: an agent whose lifecycle carried `stats`, a state with `onn` (one keystroke from `on`) and a `meta` with two unknown keys **parsed clean**, returning a machine with NO transitions at all — the declaration whose whole job is to deny undeclared transitions, silently emptied and reported valid. `.meta` was checked for the #4909 open-slot case and is CLOSED: the hand-written `StateNodeConfig` type declares exactly its four keys (passthrough would open the Zod while `tsc` stayed shut), nothing in the repo reads any `meta` key, and the prior behaviour was strip — an author's `meta` arrived as `{}` — so there was no openness to preserve. ⚠️ `ActionRef` / `GuardRef` are UNIONS: a strict branch's message does not reach the top (zod raises one `invalid_union` whose message is the literal `"Invalid input"`, with the real prescription nested in `issue.errors[]`), which `formatZodError` then flattens away — filed, not fixed here. **−1 at #4658**: the orphan `EventSchema` (`{ type, schema }`, an XState-style signal declaration nothing referenced — `StateMachineSchema` names event types as `on:` record keys) was deleted rather than converged with `kernel/events/core.zod.ts`'s envelope `EventSchema`, whose key set it did not intersect (#4535 C6). The remaining 6 sites and their verdict are unchanged | @@ -539,9 +539,9 @@ not verdicts). | `io-node-config.zod.ts` | 2 | authorable | `NotifyConfigSchema` / `HttpConfigSchema` (#4045) — the sibling contracts that validate the **open** `config` slot on flow `notify` / `http` nodes. Authored per-node, so the open-slot exemption above does not extend to them. **Strict as of #4001 批 9**; the node `config` SLOT itself stays open (ADR-0018 keeps `node.type` open, so the slot cannot be closed without closing the plugin extension point). Five `guidance` entries carry the ADR-0087 notify aliases (`to`/`subject`/`body`/`url`/`source`) | | `builtin-node-config.zod.ts` | 8 | authorable | Same family (#4045): the CRUD quartet, `screen`, `map`. Written from what the executors read rather than from the descriptors' `configSchema` literals, and reconciled bidirectionally by `builtin-node-form-zod-ledger.test.ts` — so unlike most rows here, this one already has a drift check of its own. **Strict as of #4001 批 9.** The curated tables are the `FLOW_NODE_UNKNOWN_KEY_GUIDANCE` prose from `service-automation`'s registration door, plus two entries that door never had: `recordId` (measured on CRUD nodes across the repo's own flow fixtures, read by no executor — on `delete_record` that is #3810 wearing a key that looks like a constraint) and `outputVariable` on `update_record` / `delete_record` (a documented ABSENCE, and the likeliest wrong key precisely because five sibling contracts declare it) | | `schemaless-node-config.zod.ts` | 4 | authorable | Same family, third panel (#4278): `script` / `subflow` / `decision` (+ the decision branch item) — the descriptor-schemaless nodes whose form lives in objectui's hand-written table. Written from the executors; the drift check is objectui's `flow-node-config.spec-reconciliation` test (cross-repo, via the published exports — it compares `.shape` key sets, so strictness does not move it). Since #4343 `script` and `subflow` ARE parsed at execute time (`parse-config.ts`). **Strict as of #4001 批 9 — and this is the one row in the table where strictness is the FIRST unknown-key gate, not a second one**: `registerFlow()`'s #4277 rejection derives its declared set from a descriptor `configSchema`, so it structurally skips the schemaless class. `decision` stays export-only, closed anyway; its `condition` guidance suppresses a one-edit rename to `conditions` that #4414 proves is the worse outcome | -| `webhook.zod.ts` | 1 | authorable (p) | spec-only (#3461) | -| `time-relative-trigger.zod.ts` | 1 | authorable | **Undeclared until the #4001 re-measurement, and invisible for the worst possible reason**: `TimeRelativeTriggerSchema` is written `z\n .object({`, the old textual counter matched zero sites, and a zero-site file is SKIPPED by the coverage walk as "nothing to classify". So the gate whose whole promise is "no undeclared surface" reported green over an authorable schema — the same shape as `data/driver/`, one layer subtler, because this time the file was not hidden by the walk but by the counter feeding it. Classification is not a guess: the file's own `@example` blocks author it by hand into a flow start node (`config: { timeRelative: { object, dateField, offsetDays, filter } }`), which is the authoring door. A stripped key here means the sweep silently never matches — `offsetDay` for `offsetDays` returns a trigger that never fires, reported as configured | -| `flow-function.zod.ts` | 1 | authorable | `FlowFunctionDeclarationSchema` (#4396) — the `{ handler, effect }` form of a `defineStack({ functions })` entry. Authored, but note what an undeclared key here would be: a sibling of a **live function**, not data. `defineStack`'s union already rejects a record whose `handler` is not callable, and the boot-path reader is the hand-written `normalizeFlowFunctionEntry` rather than a `.parse()` (re-validating a live handler every boot buys nothing), so strictness would bind at authoring only. Candidate on the same verify-first rule as its `*-node-config` neighbours | +| `webhook.zod.ts` | 1 | authorable | **strict as of #4001 batch 11**, and the `(p)` resolved to the opposite of what the old note ("spec-only") implied. Three parse doors, not zero: `defineWebhook()`, `defineStack({ webhooks })` via `StackSchema`, and — the one that mattered — `plugin-webhooks`' `bootstrapDeclaredWebhooks`, which re-`parse()`s every declared webhook at BOOT before materializing it into `sys_webhook`, warning and SKIPPING on failure. Which is why the ADR-0010 envelope landed in the same change rather than as a follow-up: both metadata load paths call `applyProtection` on EVERY type, so a package-loaded webhook reaches that boot parse already carrying `_packageId` / `_provenance`. `.strip` discarded them; `.strict()` alone would have converted every package-shipped webhook into a **skipped subscription after redeploy**, with one `warn` to say so. This is the envelope debt the registered-type batches paid down eight times — `webhook` is not a registered type (no `BUILTIN_METADATA_TYPE_SCHEMAS` entry), which is exactly why the invariant test that guards those never looked here. ⚠️ Strictness also rides `.extend()` onto `integration/connector.zod.ts`'s `WebhookConfigSchema` (verified against real zod, and pinned in `connector.test.ts`); its two extra keys are named in `extraKeys` — except `events`, deliberately, because it is also an alias TARGET here and listing it would walk a base-surface author through two rejections into a key the base does not accept (finding 7, arriving via hand-written `extraKeys` rather than the shape) | +| `time-relative-trigger.zod.ts` | 1 | authorable | **Undeclared until the #4001 re-measurement, and invisible for the worst possible reason**: `TimeRelativeTriggerSchema` is written `z\n .object({`, the old textual counter matched zero sites, and a zero-site file is SKIPPED by the coverage walk as "nothing to classify". So the gate whose whole promise is "no undeclared surface" reported green over an authorable schema — the same shape as `data/driver/`, one layer subtler, because this time the file was not hidden by the walk but by the counter feeding it. Classification is not a guess: the file's own `@example` blocks author it by hand into a flow start node (`config: { timeRelative: { object, dateField, offsetDays, filter } }`), which is the authoring door. A stripped key here means the sweep silently never matches — `offsetDay` for `offsetDays` returns a trigger that never fires, reported as configured. **Strict as of #4001 batch 11**, and closing it turned up one thing the triage did not predict: this schema is `safeParse`d at BIND time by `TimeRelativeTriggerPlugin` (`time-relative-trigger.ts`), not only at authoring — so the descriptor sitting under the deliberately-OPEN node `config` slot (ADR-0018) now has exactly one gate, and it is a runtime one. The behaviour change is the campaign's whole thesis in miniature: `{ …valid, offsetDay: 7 }` used to bind a sweep that ran daily with the author's narrowing discarded; it now refuses to bind and the plugin's warning carries the key and the rename | +| `flow-function.zod.ts` | 1 | authorable | `FlowFunctionDeclarationSchema` (#4396) — the `{ handler, effect }` form of a `defineStack({ functions })` entry. Authored, but note what an undeclared key here would be: a sibling of a **live function**, not data. `defineStack`'s union already rejects a record whose `handler` is not callable, and the boot-path reader is the hand-written `normalizeFlowFunctionEntry` rather than a `.parse()` (re-validating a live handler every boot buys nothing), so strictness would bind at authoring only. **Strict as of #4001 batch 11**, and the verify-first pass confirmed that reading exactly — stated in the code rather than left implied, because a tightening must not claim reach it does not have. It is still worth having for the reason the reading first made it look pointless: `normalizeFlowFunctionEntry` takes TWO keys and ignores the rest **by construction**, so a misspelled `effect` was dropped at the schema and then not looked for by the reader — and the failure runs the quiet way. The function registers, runs, and its writes are counted as none, which is precisely what keeps #4354's broken-sweep query (`selected > 0 AND acted = 0 AND unmeasured = 0`) silent on the one run that needed it | `trigger-registry.zod.ts` had a row here (11 sites, "mixed — descriptors are code-registered (wire-ish); bindings authored") until #4499 deleted the file: all 11 sites were the third connector-vocabulary declaration (`ConnectorSchema` / `Authentication*` / `Operation*` / `ConnectorInstance`), and the old row's classification was optimistic twice over — nothing was ever code-registered against these descriptors and no binding was ever authored. The engine registers against `integration/connector.zod.ts` (ADR-0097), which keeps its own row. @@ -606,40 +606,50 @@ classes; where it does, the split is stated. **Only the authorable half is in th 2026-08-03 ruling's forced scope** — wire/open rows are listed so the arithmetic is complete and so nobody re-triages them from scratch next batch. -#### `automation/` — 42 strip of 75 +#### `automation/` — 33 strip of 75 | File | Strip | Sites | Class | Batch | |---|---|---|---|---| | `execution.zod.ts` | 13 | 13 | wire | **out of scope** — engine-emitted run state; the ledger row already says "never strict" | | `etl.zod.ts` | 10 | 10 | mixed | 7 authorable (`ETLSource` + `.incremental`, `ETLDestination`, `ETLTransformation`, `ETLPipeline` + `.retry` + `.notifications`), 3 wire (`ETLPipelineRun` + `.stats` + `.error` — run state) | -| `flow.zod.ts` | 7 | 11 | mixed | 6 authorable (`FlowNode.connectorConfig` / `.position` / `.inputSchema` / `.waitEventConfig` / `.boundaryConfig`, `Flow.errorHandling`), 1 wire (`FlowVersionHistorySchema` — the ledger row already exempts it) | +| `flow.zod.ts` | 1 | 11 | wire | **batch 11 closed the 6 authorable** (`FlowNode.connectorConfig` / `.position` / `.inputSchema` / `.waitEventConfig` / `.boundaryConfig`, `Flow.errorHandling`). The 1 left is `FlowVersionHistorySchema`, which this table has exempted since it was written — **do not close it**: it is emitted on publish, not authored, so closing it makes a future emitter-side field a parse failure for whoever reads history. The exemption now also lives beside the schema and in `flow.test.ts`, because a row in a table is not where the next person to open that file will look | | `bpmn-interop.zod.ts` | 5 | 5 | wire (p) | **out of scope** — third-party BPMN import/export shapes; strictness turns an upstream addition into our parse crash | | `node-executor.zod.ts` | 4 | 4 | wire | **out of scope** — executor registration contract, code-to-code | -| `flow-function.zod.ts` | 1 | 1 | authorable | `FlowFunctionDeclarationSchema`; binds at authoring only (the boot reader is `normalizeFlowFunctionEntry`, not a `.parse()`) | -| `time-relative-trigger.zod.ts` | 1 | 1 | authorable | `TimeRelativeTriggerSchema` — **newly visible** (see its triage row); a stripped `offsetDay`/`withinDay` yields a trigger that never fires, reported as configured | -| `webhook.zod.ts` | 1 | 1 | authorable (p) | `WebhookSchema`, spec-only (#3461) | - -Three rows left this table at **批 9** (#4001), the ruling's first `automation/` -wave — `builtin-node-config.zod.ts` (8), `schemaless-node-config.zod.ts` (4) and -`io-node-config.zod.ts` (2), all reaching zero strip. The reverse pin fired on -all three before the rows were removed, which is the only evidence that a -deletion here is bookkeeping rather than a guess. - -Two more left it at **批 10** — `control-flow.zod.ts` (5) and -`state-machine.zod.ts` (6), same reverse-pin evidence. Worth recording how the -two waves met, because it is the failure mode this table is most exposed to: -both PRs deleted their own rows and decremented this header by their own count, -so git merged the ROWS cleanly (they do not overlap) and left the header -conflicted — and the subtotal line below it, which conflicts with nothing, -merged clean while being **wrong on both branches**. Neither number was a -mistake in isolation; each was computed against one wave's deletions. The -header is therefore recomputed from the surviving rows rather than resolved in -favour of either side, and `check:strictness-ledger`'s header arithmetic is -what settles it. A clean-looking merge here is not evidence of anything. - -**Authorable strip in `automation/`: 16 of 42** (was 41 of 67 before the two -waves). What remains of the ruling's "known main body" is `etl` 7, `flow` 6, -and one each from `flow-function` / `time-relative-trigger` / `webhook`. + +Eight rows have left this table across three waves of the ruling's `automation/` +main body, each one on reverse-pin evidence — the row was deleted because the +gate went red on it still being there, not because someone remembered: + +| wave | rows removed | other change | +|---|---|---| +| **批 9** (#4925) | `builtin-node-config` (8) · `schemaless-node-config` (4) · `io-node-config` (2) | — | +| **批 10** (#4973) | `control-flow` (5) · `state-machine` (6) | — | +| **批 11** (#4974) | `flow-function` (1) · `time-relative-trigger` (1) · `webhook` (1) | `flow.zod.ts` 7 → 1 | + +**How those waves met is worth recording, because it is the failure mode this +table is most exposed to.** Each PR deleted its own rows and decremented this +header by its own count. Git therefore merged the ROWS cleanly — they do not +overlap — and left only the header conflicted, while the subtotal line below it, +which conflicts with nothing, **merged clean and wrong**. No number was a mistake +in isolation: each was correct against the branch that computed it. + +It has now happened three times in one day. 批 10 recorded it against 批 9's +header; 批 11 then merged and its subtotal (`etl` 7 + `state-machine` 6 + +`control-flow` 5) and 批 10's (`etl` 7 + `flow` 6 + three singles) were each +right against their own branch and both wrong against the merge. So the rule is +mechanical rather than remembered: **the header and the subtotal are recomputed +from the surviving rows, never resolved in favour of a side**, and +`check:strictness-ledger`'s arithmetic is what settles it. A clean-looking merge +here is evidence of nothing. + +**Authorable strip in `automation/`: 7 of 33** (was 41 of 67 before the three +waves). What is left of the ruling's "known main body" is **`etl.zod.ts` alone** +— `ETLSource` + `.incremental`, `ETLDestination`, `ETLTransformation`, +`ETLPipeline` + `.retry` + `.notifications`. The other 26 strip sites here are +wire and out of the ruling's forced scope: `execution` 13, `bpmn-interop` 5, +`node-executor` 4, `etl`'s own 3 run-state shapes, and `flow.zod.ts`'s last site +`FlowVersionHistorySchema` — which is why `flow` still has a row while having 0 +authorable left, and must not be read as unfinished work. #### `ui/` — 123 strip of 198 diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 64a1191624..bed1dffa6e 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -2494,6 +2494,13 @@ "automation/WaitResumePayload:signalName", "automation/WaitResumePayload:variables", "automation/WaitResumePayload:webhookPayload", + "automation/Webhook:_lock", + "automation/Webhook:_lockDocsUrl", + "automation/Webhook:_lockReason", + "automation/Webhook:_lockSource", + "automation/Webhook:_packageId", + "automation/Webhook:_packageVersion", + "automation/Webhook:_provenance", "automation/Webhook:description", "automation/Webhook:headers", "automation/Webhook:isActive", @@ -2501,6 +2508,7 @@ "automation/Webhook:method", "automation/Webhook:name", "automation/Webhook:object", + "automation/Webhook:protection", "automation/Webhook:secret", "automation/Webhook:timeoutMs", "automation/Webhook:triggers", @@ -4169,6 +4177,13 @@ "integration/RetryConfig:retryOnNetworkError", "integration/RetryConfig:retryableStatusCodes", "integration/RetryConfig:strategy", + "integration/WebhookConfig:_lock", + "integration/WebhookConfig:_lockDocsUrl", + "integration/WebhookConfig:_lockReason", + "integration/WebhookConfig:_lockSource", + "integration/WebhookConfig:_packageId", + "integration/WebhookConfig:_packageVersion", + "integration/WebhookConfig:_provenance", "integration/WebhookConfig:description", "integration/WebhookConfig:events", "integration/WebhookConfig:headers", @@ -4177,6 +4192,7 @@ "integration/WebhookConfig:method", "integration/WebhookConfig:name", "integration/WebhookConfig:object", + "integration/WebhookConfig:protection", "integration/WebhookConfig:secret", "integration/WebhookConfig:signatureAlgorithm", "integration/WebhookConfig:timeoutMs", diff --git a/packages/spec/src/automation/flow-function.test.ts b/packages/spec/src/automation/flow-function.test.ts index a0a0ddbab5..c2f95ca31b 100644 --- a/packages/spec/src/automation/flow-function.test.ts +++ b/packages/spec/src/automation/flow-function.test.ts @@ -122,3 +122,64 @@ describe('defineStack({ functions }) — the authoring surface (#4396)', () => { } as never)).toThrow(); }); }); + +// #4001 batch 11. Worth being precise about WHERE this binds, because the +// campaign's rule is that a tightening claims no reach it does not have: +// authoring only. `defineStack` parses the entry; the boot path +// (`AppPlugin` / `hook-binder`) reads it with the hand-written +// `normalizeFlowFunctionEntry` instead. +// +// Which is exactly why it is not redundant. That reader takes TWO keys and +// ignores everything else by construction, so a misspelled `effect` was +// dropped at the schema and then not looked for — and the failure is the quiet +// direction: the function registers, runs, and its writes are counted as none, +// keeping #4354's broken-sweep query silent on the one run that needed it. +describe('unknown keys are rejected, not stripped (#4001 batch 11)', () => { + const base = { + manifest: { id: 'com.example.demo', name: 'demo', version: '1.0.0', type: 'app' as const }, + }; + const unknownKeyIssue = (value: unknown) => { + const result = FlowFunctionDeclarationSchema.safeParse(value); + expect(result.success).toBe(false); + return result.error!.issues.find((i) => i.code === 'unrecognized_keys'); + }; + + it('rejects a misspelled `effect` instead of silently reading it as pure', () => { + const issue = unknownKeyIssue({ handler: () => 1, efect: 'writes' }); + expect(issue!.message).toContain('`functions` entry'); + expect(issue!.message).toContain('`efect` → `effect`'); + }); + + it('points the short words for "the callable" at `handler`', () => { + for (const key of ['fn', 'callback', 'execute']) { + expect(unknownKeyIssue({ handler: () => 1, [key]: () => 2 })!.message) + .toContain(`\`${key}\` → \`handler\``); + } + }); + + it('explains that a function is named by its MAP KEY, not by a `name` inside', () => { + const issue = unknownKeyIssue({ handler: () => 1, name: 'scoreLead' }); + expect(issue!.message).toContain('named by its KEY'); + // A rename would be wrong: `name` is real on the ARRAY form, and pointing + // at a declared key of this record would be inventing one. + expect(issue!.message).not.toContain('`name` → '); + }); + + it('binds through defineStack — the authoring door this actually gates', () => { + expect(() => defineStack({ + ...base, + functions: { syncBilling: { handler: () => ({ ok: true }), efect: 'writes' } }, + } as never)).toThrow(); + }); + + it('leaves the two spellings an author actually writes alone', () => { + expect(FlowFunctionDeclarationSchema.safeParse({ handler: () => 1 }).success).toBe(true); + expect(FlowFunctionDeclarationSchema.safeParse({ handler: () => 1, effect: 'writes' }).success).toBe(true); + // The array form keeps its own shape — `name`/`packageId` live there, not + // on this record, and strictness here must not reach across. + expect(defineStack({ + ...base, + functions: [{ name: 'syncBilling', handler: () => ({ ok: true }), packageId: 'p', effect: 'writes' }], + })).toBeTruthy(); + }); +}); diff --git a/packages/spec/src/automation/flow-function.zod.ts b/packages/spec/src/automation/flow-function.zod.ts index 59fc7a7fb8..dacbc0558c 100644 --- a/packages/spec/src/automation/flow-function.zod.ts +++ b/packages/spec/src/automation/flow-function.zod.ts @@ -51,6 +51,7 @@ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; +import { strictObject } from '../shared/strict-object'; /** * What a `script`-node function does to data (#4396). @@ -101,8 +102,58 @@ export const DEFAULT_FLOW_FUNCTION_EFFECT: FlowFunctionEffect = 'pure'; * a function anything — it changes what the platform *reports* about the runs * that call it, which is the whole point: an undeclared writer is counted as * having written nothing. + * + * ## Where strictness on this shape actually binds (#4001 batch 11) + * + * At AUTHORING time, and only there — worth stating precisely, because the + * campaign's rule is that a tightening claims no reach it does not have. + * `defineStack({ functions })` parses this through + * {@link FlowFunctionEntrySchema}, so `{ handler, efect: 'writes' }` is refused + * where it was written. The BOOT path does not re-parse: `AppPlugin` and + * `hook-binder` read entries with the hand-written + * {@link normalizeFlowFunctionEntry} (re-validating a live callable every boot + * buys nothing), so a stack that never went through `defineStack` is not gated + * by this. + * + * That is exactly why the strictness matters rather than being redundant. + * `normalizeFlowFunctionEntry` reads TWO keys and ignores the rest by + * construction, so before this change a misspelled `effect` was dropped at the + * schema and then *not looked for* by the reader — and the failure is the quiet + * direction: the function is registered, it runs, and its writes are counted as + * none, so #4354's broken-sweep query stays silent on the one flow that needed + * it. (A misspelled `effect` VALUE — `'write'` — already fails loudly-ish: + * `normalizeFlowFunctionEntry` degrades it to `'writes'` and surfaces the raw + * string. A misspelled KEY had no such backstop.) */ -export const FlowFunctionDeclarationSchema = lazySchema(() => z.object({ +export const FlowFunctionDeclarationSchema = lazySchema(() => strictObject({ + surface: 'this `functions` entry', + aliases: { + // A different word for "the callable", from the shapes an author has just + // been writing: hooks/jobs/endpoints in this repo all name theirs `handler` + // too, but `fn`/`callback`/`execute`/`run` are what the short forms in + // other products call it, and none is within edit distance of `handler`. + fn: 'handler', + func: 'handler', + callback: 'handler', + execute: 'handler', + run: 'handler', + // `effects` (plural) is one edit away and the distance fallback gets it; + // `writes`/`sideEffects` are the concept named instead of the key. + sideEffects: 'effect', + writes: 'effect', + }, + guidance: { + name: + 'A function is named by its KEY in the `functions` map (`functions: { scoreLead: {…} }`), ' + + 'not by a `name` inside the entry. The array form `functions: [{ name, handler }]` does ' + + 'take one — but that is the array entry\'s own shape, not this record.', + }, + history: + 'Until #4001 these were dropped silently — and `normalizeFlowFunctionEntry` reads only ' + + '`handler`/`effect` by construction, so a misspelled `effect` was discarded twice over: ' + + 'the function still registered, still ran, and its writes were still counted as none, ' + + 'which is what keeps #4354\'s broken-sweep alert quiet on the run that needed it.', +}, { handler: z.function().describe('The function invoked by name (a `script` node, a string-named Hook/Action handler)'), effect: FlowFunctionEffectSchema.default(DEFAULT_FLOW_FUNCTION_EFFECT) .describe("What the function does to data — omit for the pure default"), diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index ea83c03070..c3a0e3bf17 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -1306,4 +1306,165 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { .toContain('`is_input` → `isInput`'); }); }); + + // ── batch 11: the INNER blocks ──────────────────────────────────────────── + // + // Closing the four outer shells above left six nested authoring blocks on + // zod's default `.strip`. Same defect, one layer in — a guard put where the + // author who wrote it was standing. + // + // What those six were actually hiding is worth stating, because it is not the + // obvious case: a slip on a REQUIRED key was always loud (the key then reads + // as missing). `.strip` swallowed the OPTIONAL half — the mapped input map, + // the retry budget, `interrupting: false`, `required: true` — i.e. precisely + // the keys an author adds to CONSTRAIN behaviour, silently replaced by a + // permissive default. + describe('the nested authoring blocks (batch 11)', () => { + const node = (extra: Record) => ({ id: 'n1', type: 'script', label: 'N', ...extra }); + + it('connectorConfig: rejects an undeclared key and points input synonyms at `input`', () => { + const issue = unknownKeyIssue(FlowNodeSchema, node({ + connectorConfig: { connectorId: 'rest', actionId: 'get', params: {} }, + })); + expect(issue!.message).toContain("connector_action node's `connectorConfig`"); + expect(issue!.message).toContain('`params` → `input`'); + }); + + it('connectorConfig: the silent case was the OPTIONAL half, not the ids', () => { + // Before this change, `{ connectorId, actionId, params }` parsed clean and + // the executor dispatched `input ?? {}` — a successful call carrying + // nothing. A slip on a REQUIRED id was never silent (it reads as missing), + // which is why this block's history names the input map and not the ids. + const result = FlowNodeSchema.safeParse(node({ + connectorConfig: { connectorID: 'rest', actionId: 'get' }, + })); + expect(result.success).toBe(false); + expect(JSON.stringify(result.error!.issues)).toContain('connectorId'); + }); + + it('position: rejects a third coordinate rather than dropping it at (0, 0)', () => { + const issue = unknownKeyIssue(FlowNodeSchema, node({ position: { x: 1, y: 2, z: 3 } })); + expect(issue!.message).toContain("node's canvas `position`"); + expect(issue!.message).toContain('`z`'); + }); + + it('inputSchema: an `optional` key gets the POLARITY, not a bare rename', () => { + const issue = unknownKeyIssue(FlowNodeSchema, node({ + inputSchema: { url: { type: 'string', optional: true } }, + })); + // A bare "did you mean `required`" would be a confidently wrong + // prescription — `optional: true` is `required: FALSE`. The rejection + // has to say which way to flip the value. + expect(issue!.message).toContain('required: false'); + expect(issue!.message).not.toContain('`optional` → `required`'); + }); + + it('waitEventConfig: renames `signal`, and sends `timeout` to `timerDuration` — never to the tombstone', () => { + const issue = unknownKeyIssue(FlowNodeSchema, node({ + type: 'wait', + waitEventConfig: { eventType: 'signal', signal: 'paid', timeout: 60_000 }, + })); + expect(issue!.message).toContain('`signal` → `signalName`'); + expect(issue!.message).toContain('`timerDuration`'); + // `timeoutMs` is a #4158 tombstone. Pointing a typo at a REMOVED key is + // the `triggerPhrase → triggerPhrases` chain (ledger finding 7): the + // author follows the advice straight into a second rejection. + expect(issue!.message).not.toContain('→ `timeoutMs`'); + expect(issue!.message).not.toContain('→ `onTimeout`'); + }); + + it('boundaryConfig: translates BPMN\'s own attribute names', () => { + const issue = unknownKeyIssue(FlowNodeSchema, node({ + type: 'boundary_event', + boundaryConfig: { attachedToRef: 'n0', eventType: 'error', cancelActivity: true }, + })); + expect(issue!.message).toContain('`attachedToRef` → `attachedToNodeId`'); + expect(issue!.message).toContain('`cancelActivity` → `interrupting`'); + }); + + it('errorHandling: `backoffMs` is the sibling retry policy\'s converged spelling (#4661)', () => { + const issue = unknownKeyIssue(FlowSchema, { + ...minimalFlow, + errorHandling: { strategy: 'retry', maxRetries: 3, backoffMs: 5000 }, + }); + expect(issue!.message).toContain("flow's `errorHandling` block"); + expect(issue!.message).toContain('`backoffMs` → `retryDelayMs`'); + }); + + it('errorHandling: `maxAttempts` gets the off-by-one, not a rename', () => { + const issue = unknownKeyIssue(FlowSchema, { + ...minimalFlow, + errorHandling: { strategy: 'retry', maxAttempts: 3 }, + }); + // Renaming alone would silently run one attempt FEWER than asked for: + // RetryConfig's `maxAttempts` counts the first try, `maxRetries` does not. + expect(issue!.message).not.toContain('`maxAttempts` → `maxRetries`'); + expect(issue!.message).toContain('maxAttempts - 1'); + }); + + it('errorHandling: the `strategy: retry` refinement still runs after the block is strict', () => { + // The `.superRefine` chains off `strictObject(...)` now. Losing it would + // re-open #4247's zero-attempt "retry", and nothing else here would tell. + const result = FlowSchema.safeParse({ ...minimalFlow, errorHandling: { strategy: 'retry' } }); + expect(result.success).toBe(false); + expect(JSON.stringify(result.error!.issues)).toContain('requires `maxRetries` >= 1'); + }); + + it('every key the six blocks declare still parses', () => { + const parsed = FlowNodeSchema.parse(node({ + type: 'connector_action', + connectorConfig: { connectorId: 'c', actionId: 'a', input: { k: 1 } }, + position: { x: 1, y: 2 }, + inputSchema: { p: { type: 'string', required: true, description: 'd' } }, + waitEventConfig: { eventType: 'timer', timerDuration: 'PT1H', signalName: 's' }, + boundaryConfig: { + attachedToNodeId: 'n0', eventType: 'timer', interrupting: false, + errorCode: 'E', timerDuration: 'PT5M', signalName: 's', + }, + })); + expect(parsed.connectorConfig!.input).toEqual({ k: 1 }); + const flow = FlowSchema.parse({ + ...minimalFlow, + errorHandling: { + strategy: 'retry', maxRetries: 2, retryDelayMs: 10, backoffMultiplier: 2, + maxRetryDelayMs: 100, jitter: true, + }, + }); + expect(flow.errorHandling!.jitter).toBe(true); + }); + }); + + // The two shapes this file deliberately leaves open. Asserted, not assumed, + // so the next sweep reads a test rather than reaching for `strictObject`. + describe('deliberately still open', () => { + it('the node `config` slot stays open (ADR-0018 plugin node-type namespace)', () => { + const parsed = FlowNodeSchema.parse({ + id: 'n1', type: 'some_plugin_node', label: 'P', + config: { aKeyOnlyThatPluginDeclares: true }, + }); + expect((parsed.config as Record).aKeyOnlyThatPluginDeclares).toBe(true); + }); + + it('FlowVersionHistorySchema stays open — it is emitted, not authored', () => { + // Every other object site in flow.zod.ts is closed, so this reads like + // the last hold-out. It is the file's only WIRE shape (the ledger row has + // exempted it since it was written): closing it would turn a future + // emitter-side field into a parse failure for whoever reads history. + const parsed = FlowVersionHistorySchema.parse({ + flowName: 'f', version: 1, definition: minimalFlow, + createdAt: '2026-08-03T00:00:00.000Z', + aFieldSomeFutureWriterStamps: true, + }); + expect(parsed.flowName).toBe('f'); + }); + + it('…but the flow INSIDE a history record is still gated by FlowSchema', () => { + const result = FlowVersionHistorySchema.safeParse({ + flowName: 'f', version: 1, + definition: { ...minimalFlow, notAKey: 1 }, + createdAt: '2026-08-03T00:00:00.000Z', + }); + expect(result.success).toBe(false); + }); + }); }); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index ff3b35aad5..fdd43409a6 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -21,6 +21,7 @@ import { strictUnknownKeyError } from '../shared/suggestions.zod'; */ import { lazySchema } from '../shared/lazy-schema'; import { retiredKey } from '../shared/retired-key'; +import { strictObject } from '../shared/strict-object'; export const FlowNodeAction = z.enum([ 'start', // Trigger 'end', // Return/Stop @@ -78,6 +79,32 @@ export const FLOW_STRUCTURAL_NODE_TYPES: readonly string[] = ['start', 'end']; * * Key lists are kept beside the schemas rather than derived from `.shape` * (bodies are allocated lazily; `flow.test.ts` drift-guards every entry). + * + * **Batch 11 closed the INNER blocks too.** Closing the four outer shells left + * six nested authoring blocks on default `.strip` — `FlowNode.connectorConfig` + * / `.position` / `.inputSchema` / `.waitEventConfig` / `.boundaryConfig` and + * `Flow.errorHandling`. That is the shape this campaign keeps re-finding: a + * guard put where the author who wrote it was standing. The outer gate rejected + * `nodee:` at the node level while `connectorConfig: { connectorId, actionId, + * params: {…} }` parsed clean and dispatched the action with **no inputs at + * all** — the executor reads `input ?? {}`, so the whole mapped payload became + * an empty object and the call succeeded against nothing. + * + * Note which cases those six were, and were not, hiding: a slip on a REQUIRED + * key (`connectorID` for `connectorId`, `attachedToRef` for `attachedToNodeId`) + * was always loud, because the required key then reads as missing. What + * `.strip` swallowed is the OPTIONAL half — the input map, the retry budget, + * `interrupting: false`, `required: true` — i.e. exactly the keys an author adds + * to CONSTRAIN behaviour, dropped back to a permissive default without a word. + * + * They use {@link strictObject}, whose candidate list is read from the shape + * itself, so these six need no drift-guard entry (and adding one would be the + * second copy of the truth the helper exists to delete). + * + * Deliberately still open, both re-confirmed here rather than left to be + * rediscovered: the node `config` slot (above), and + * {@link FlowVersionHistorySchema} at the foot of this file (wire — see its own + * note). */ /** Keys {@link FlowVariableSchema} declares (drift-guarded by flow.test.ts). */ @@ -174,24 +201,82 @@ export const FlowNodeSchema = lazySchema(() => z.object({ * declared here turned a no-input action into a load failure nothing * downstream asked for. */ - connectorConfig: z.object({ - connectorId: z.string().describe('Registered connector name'), - actionId: z.string().describe('Action key declared by the connector'), - input: z.record(z.string(), z.unknown()).optional().describe('Mapped inputs for the action'), - }).optional(), + connectorConfig: strictObject( + { + surface: "this connector_action node's `connectorConfig`", + // The aliases are the four words the neighbouring surfaces use for the + // same thing: `config.inputs` on a script node, `input` on the connector + // ACTION descriptor, `params`/`parameters` in every integration product + // an author (or an AI) imports vocabulary from. `inputs` is left to the + // distance fallback — it is one edit away and gets there for free. + aliases: { + params: 'input', + parameters: 'input', + arguments: 'input', + payload: 'input', + }, + history: + 'Until #4001 these were dropped silently — the block still parsed, so a whole ' + + 'mapped input map written under another word vanished and the executor ' + + 'dispatched the action with `input ?? {}`: a successful call carrying nothing.', + }, + { + connectorId: z.string().describe('Registered connector name'), + actionId: z.string().describe('Action key declared by the connector'), + input: z.record(z.string(), z.unknown()).optional().describe('Mapped inputs for the action'), + }, + ).optional(), - /** UI Position (for the canvas) */ - position: z.object({ x: z.number(), y: z.number() }).optional(), + /** + * UI Position (for the canvas). + * + * No alias table on purpose: `{ x, y }` is the same two keys React Flow (the + * Studio canvas) and BPMN DI both use, so there is no neighbouring vocabulary + * to import from — and the campaign's rule is that an alias entry is an + * empirical claim, not a precaution. The distance fallback covers `X` / `Y`. + */ + position: strictObject( + { + surface: "this node's canvas `position`", + history: + 'Until #4001 these were dropped silently — the block still parsed, so a canvas ' + + 'hint written beside x/y (a size, a third coordinate, a designer marker) was ' + + 'discarded, and the round-trip back through the designer could not tell it had ' + + 'ever been written.', + }, + { x: z.number(), y: z.number() }, + ).optional(), /** Node-level execution timeout */ timeoutMs: z.number().int().min(0).optional().describe('Maximum execution time for this node in milliseconds'), /** Node input schema declaration for Studio form generation and runtime validation */ - inputSchema: z.record(z.string(), z.object({ - type: z.enum(['string', 'number', 'boolean', 'object', 'array']).describe('Parameter type'), - required: z.boolean().default(false).describe('Whether the parameter is required'), - description: z.string().optional().describe('Parameter description'), - })).optional().describe('Input parameter schema for this node'), + inputSchema: z.record(z.string(), strictObject( + { + surface: "this node input parameter's declaration", + guidance: { + // A rename would be actively wrong here: `optional: true` and + // `required: true` are opposite claims, so pointing an author at + // `required` without saying to flip the value is the "confidently + // wrong prescription" this campaign has shipped before. Say the flip. + optional: + 'There is no `optional` on an input parameter — the polarity is the other way ' + + 'round. Write `required: false` (which is also the default, so the key can just ' + + 'be dropped); `optional: true` is `required: false`, and `optional: false` is ' + + '`required: true`.', + }, + history: + 'Until #4001 these were dropped silently — the declaration still parsed, so a ' + + 'parameter constrained under a word we do not declare (`optional: false`) came ' + + 'back UNconstrained: `required` fell to its `false` default, and the engine\'s ' + + 'pre-execution check (`validateNodeInputSchemas`) then had nothing to require.', + }, + { + type: z.enum(['string', 'number', 'boolean', 'object', 'array']).describe('Parameter type'), + required: z.boolean().default(false).describe('Whether the parameter is required'), + description: z.string().optional().describe('Parameter description'), + }, + )).optional().describe('Input parameter schema for this node'), // `outputSchema` REMOVED (#3896 audit close-out): declared, never validated — // no engine path checked node outputs against it (ledger: dead). @@ -207,7 +292,30 @@ export const FlowNodeSchema = lazySchema(() => z.object({ * Defines what external event or condition should resume the paused execution. * Industry alignment: BPMN Intermediate Catch Events, Temporal Signals. */ - waitEventConfig: z.object({ + waitEventConfig: strictObject({ + surface: "this wait node's `waitEventConfig`", + aliases: { + // Different WORD, same intent — none of these is within edit distance of + // the key it means. `duration`/`delay` are what the two neighbouring + // retry/timer shapes in this repo call a millisecond span. + event: 'eventType', + signal: 'signalName', + duration: 'timerDuration', + delay: 'timerDuration', + }, + guidance: { + // Deliberately NOT an alias to `timeoutMs`: that key is a tombstone + // (below) and pointing a typo at a removed key is how the campaign's own + // helper once told an author to write something that gets rejected next. + timeout: + '`wait` has no timeout — nothing has ever failed or resumed a wait on a deadline ' + + '(#4158 retired the two keys that claimed one). Use `timerDuration`: it accepts a ' + + 'bare number as milliseconds, so `timerDuration: 60000` is a 60s wait.', + }, + history: + 'Until #4001 these were dropped silently — the block still parsed, so a wait node ' + + 'whose resume condition the author spelled slightly wrong waited on nothing.', + }, { /** Type of event to wait for */ eventType: z.enum(['timer', 'signal', 'webhook', 'manual', 'condition']) .describe('What kind of event resumes the execution'), @@ -256,7 +364,28 @@ export const FlowNodeSchema = lazySchema(() => z.object({ * Attaches an event handler to a host activity node (BPMN Boundary Event pattern). * Industry alignment: BPMN Boundary Error/Timer/Signal Events. */ - boundaryConfig: z.object({ + boundaryConfig: strictObject({ + surface: "this boundary event's `boundaryConfig`", + aliases: { + // This block's own doc claims BPMN alignment, and `bpmn-interop.zod.ts` + // exists so a third-party definition can be imported — so BPMN's OWN + // attribute names are exactly the words an author arrives with. + // `attachedToRef` and `cancelActivity` are verbatim BPMN 2.0 spellings + // of the two keys below, and neither is within edit distance of it. + attachedToRef: 'attachedToNodeId', + attachedTo: 'attachedToNodeId', + hostNodeId: 'attachedToNodeId', + cancelActivity: 'interrupting', + event: 'eventType', + signal: 'signalName', + duration: 'timerDuration', + }, + history: + 'Until #4001 these were dropped silently — the block still parsed, so BPMN\'s ' + + '`cancelActivity: false` was discarded and `interrupting` fell to its `true` ' + + 'default: an event the author declared NON-interrupting cancelled the host ' + + 'activity anyway.', + }, { /** ID of the host node this boundary event is attached to */ attachedToNodeId: z.string().describe('Host node ID this boundary event monitors'), /** Type of boundary event */ @@ -490,7 +619,50 @@ export const FlowSchema = lazySchema(() => z.object({ * `'continue'` ignore them (a fully spelled-out block under `'fail'` is * common and stays legal). */ - errorHandling: z.object({ + errorHandling: strictObject({ + surface: "this flow's `errorHandling` block", + aliases: { + // Every one of these is a real, in-repo spelling of the same knob on a + // NEIGHBOURING retry surface — which is what makes this table an + // empirical claim rather than a guess about typos: + // `shared/retry-policy.zod.ts` (#4661, job.retryPolicy + a try_catch + // node's `retry`) → `backoffMs`, and it TOMBSTONED `retryDelayMs` + // as "the automation-side spelling", so an author who learned the + // converged word and brings it here is being punished for reading + // the newer file. + // `integration/connector.zod.ts` RetryConfig → `initialDelayMs`, + // `maxDelayMs`. + // `retries`/`attempts` are the plain-English forms; `onError` is n8n's + // word for the strategy switch. + backoffMs: 'retryDelayMs', + initialDelayMs: 'retryDelayMs', + maxDelayMs: 'maxRetryDelayMs', + retries: 'maxRetries', + attempts: 'maxRetries', + onError: 'strategy', + }, + guidance: { + // NOT an alias. `maxAttempts` (connector RetryConfig) counts the FIRST + // attempt; `maxRetries` counts the ones after it. A bare rename would + // silently change the number's meaning by one — the exact shape of the + // four wrong prescriptions this campaign shipped and had to withdraw. + maxAttempts: + '`maxAttempts` is the connector/RetryConfig spelling and INCLUDES the first attempt; ' + + 'flow `errorHandling` counts retries AFTER it. Write `maxRetries: ` ' + + '— renaming the key alone would quietly run one attempt fewer than you asked for.', + fallback: + 'There is no fallback node on `errorHandling` (`fallbackNodeId` was removed in 17, ' + + '#3896 — the engine never read it). Draw a per-node FAULT EDGE from the failing node ' + + 'to the handler node instead.', + fallbackNode: + 'There is no fallback node on `errorHandling` (`fallbackNodeId` was removed in 17, ' + + '#3896 — the engine never read it). Draw a per-node FAULT EDGE from the failing node ' + + 'to the handler node instead.', + }, + history: + 'Until #4001 these were dropped silently — the block still parsed, so a retry budget ' + + 'or backoff the author configured was replaced by this block\'s defaults without a word.', + }, { strategy: z.enum(['fail', 'retry', 'continue']).default('fail').describe('How to handle node execution errors'), // Default 0 = "no retries", which is the right reading for the two // strategies that never retry. Under `strategy: 'retry'` it would instead @@ -584,6 +756,27 @@ export type FlowEdgeParsed = z.infer; * Tracks historical versions of flow definitions for rollback support. * * Industry alignment: Salesforce Flow Versions, n8n Workflow History. + * + * ## Deliberately NOT `.strict()` — stop here (#4001 batch 11) + * + * Every other object site in this module is closed, so the next sweep will read + * this one as the last hold-out and reach for `strictObject`. It is not a + * hold-out: it is the only WIRE shape in the file, and the strictness ledger's + * `automation/flow.zod.ts` row has exempted it since the row was written. + * + * Nobody authors a version-history record. The engine/Studio EMIT one when a + * flow is published — `flowName` / `version` / `createdAt` / `createdBy` are + * stamped by the writer, not typed by a person — so the asymmetry that makes + * strictness right everywhere else (author writes it, a dropped key is a silent + * defect they own) does not hold: here an added field is *our* enrichment, and + * closing this shape would turn a future emitter-side addition into a parse + * failure for anyone reading history they were handed. Same reasoning, and the + * same verdict, as `HookContextSchema` and the `execution.zod.ts` run-state + * envelopes. + * + * `definition` is the authored half, and it is already strict — it references + * {@link FlowSchema}, so the flow inside a history record is validated by the + * gate above, exactly where the author's keys are. */ export const FlowVersionHistorySchema = lazySchema(() => z.object({ flowName: z.string().describe('Flow machine name'), diff --git a/packages/spec/src/automation/time-relative-trigger.test.ts b/packages/spec/src/automation/time-relative-trigger.test.ts index 9f45959516..bd2b705555 100644 --- a/packages/spec/src/automation/time-relative-trigger.test.ts +++ b/packages/spec/src/automation/time-relative-trigger.test.ts @@ -84,3 +84,68 @@ describe('TimeRelativeTriggerSchema', () => { expect(TIME_RELATIVE_DEFAULT_MAX_RECORDS).toBeGreaterThan(0); }); }); + +// #4001 batch 11. This descriptor was off the strictness map entirely until the +// 2026-08-03 re-measurement: its single site is written `z\n .object({`, the +// old textual counter read zero sites, and a zero-site file is SKIPPED by the +// coverage walk as "nothing to classify" — so the gate that promises "no +// undeclared authorable surface" printed green over it. +// +// It also sits BELOW the deliberately-open node `config` slot (ADR-0018), so +// the flow gate cannot see inside it. This schema is the only gate there is. +describe('unknown keys are rejected, not stripped (#4001 batch 11)', () => { + const unknownKeyIssue = (value: unknown) => { + const result = TimeRelativeTriggerSchema.safeParse(value); + expect(result.success).toBe(false); + return result.error!.issues.find((i) => i.code === 'unrecognized_keys'); + }; + + it('rejects the singular slip that used to bind a sweep matching nothing', () => { + // The exact shape the ledger row names: `offsetDay` next to a VALID + // mode key. Before this, the descriptor parsed, the plugin bound a + // daily sweep, and the author's narrowing was gone — a trigger that + // never fires on the day it was configured for, reported as configured. + const issue = unknownKeyIssue({ + object: 'contracts', dateField: 'end_date', withinDays: 30, offsetDay: 7, + }); + expect(issue!.message).toContain('`config.timeRelative` descriptor'); + expect(issue!.message).toContain('`offsetDay` → `offsetDays`'); + }); + + it('points ObjectQL / record-change vocabulary at the descriptor\'s keys', () => { + expect(unknownKeyIssue({ object: 'c', field: 'due_date', withinDays: 1 })!.message) + .toContain('`field` → `dateField`'); + expect(unknownKeyIssue({ object: 'c', dateField: 'd', withinDays: 1, filters: {} })!.message) + .toContain('`filters` → `filter`'); + expect(unknownKeyIssue({ objectName: 'c', dateField: 'd', withinDays: 1 })!.message) + .toContain('`objectName` → `object`'); + }); + + it('sends the two sibling keys to the layer that owns them', () => { + // `schedule` and `runAs` are real and adjacent — a rename would be + // wrong; the prescription is where they belong. + expect(unknownKeyIssue({ object: 'c', dateField: 'd', withinDays: 1, schedule: {} })!.message) + .toContain("START node's `config`"); + expect(unknownKeyIssue({ object: 'c', dateField: 'd', withinDays: 1, runAs: 'system' })!.message) + .toContain('FLOW-level'); + }); + + it('still accepts every key it declares, in both windowing modes', () => { + expect(TimeRelativeTriggerSchema.parse({ + object: 'contracts', dateField: 'end_date', offsetDays: [60, 30, 7], + filter: { status: 'active' }, maxRecords: 500, + }).maxRecords).toBe(500); + expect(TimeRelativeTriggerSchema.parse({ + object: 'hr_document', dateField: 'expires_on', withinDays: 30, + }).withinDays).toBe(30); + }); + + it('keeps the exactly-one-mode refinement after the shape went strict', () => { + // The `.refine` chains off `strictObject(...)` now; losing it would let + // a descriptor with neither mode (or both) through. + expect(TimeRelativeTriggerSchema.safeParse({ object: 'c', dateField: 'd' }).success).toBe(false); + expect(TimeRelativeTriggerSchema.safeParse({ + object: 'c', dateField: 'd', withinDays: 1, offsetDays: [1], + }).success).toBe(false); + }); +}); diff --git a/packages/spec/src/automation/time-relative-trigger.zod.ts b/packages/spec/src/automation/time-relative-trigger.zod.ts index 760a3431eb..618386b049 100644 --- a/packages/spec/src/automation/time-relative-trigger.zod.ts +++ b/packages/spec/src/automation/time-relative-trigger.zod.ts @@ -2,6 +2,7 @@ import { z } from 'zod'; import { lazySchema } from '../shared/lazy-schema'; +import { strictObject } from '../shared/strict-object'; /** * Time-Relative Trigger Protocol @@ -66,10 +67,69 @@ const MACHINE_NAME = /^[a-z_][a-z0-9_]*$/; * Declarative descriptor for a time-relative trigger. Lives on a flow's start * node under `config.timeRelative`. Exactly ONE windowing mode — `withinDays` * (a range) or `offsetDays` (discrete thresholds) — must be set. + * + * ## Closed against unknown keys (#4001 batch 11, ADR-0078) + * + * This schema was off the strictness map entirely until the 2026-08-03 + * re-measurement, and for a reason worth leaving here: its single site is + * written `z\n .object({`, the ledger's old textual counter matched zero sites, + * and a zero-site file is SKIPPED by the coverage walk as "nothing to classify". + * The gate whose whole promise is *no undeclared authorable surface* printed + * green over this file. The counter now reads the AST (#4852) — but the schema + * it uncovered was still on default `.strip`, which is what this closes. + * + * The stakes here are higher than a dropped key usually is, because the + * descriptor lands on the node `config` slot, which is open BY DESIGN + * (ADR-0018) — so the outer flow gate cannot see a typo inside it, and the only + * gate that can is this one. And the failure is silent in both directions: + * {@link TimeRelativeTrigger} is `safeParse`d at BIND time by + * `TimeRelativeTriggerPlugin`, so before this change `offsetDay` (singular) or + * `withinDay` next to a valid mode key bound a sweep that ran daily, matched + * with the author's narrowing key discarded, and reported itself configured. + * After it, the bind refuses and the warning names the key and the fix. */ export const TimeRelativeTriggerSchema = lazySchema(() => - z - .object({ + strictObject( + { + surface: "this flow start node's `config.timeRelative` descriptor", + aliases: { + // Different WORD, same intent — the words the neighbouring authoring + // surfaces use. `object`/`filter` are already the ObjectQL spellings + // (`objectName`/`filters` graduated to ADR-0087 conversions at 17), so + // an author arriving from a CRUD node's `config` brings exactly these. + objectName: 'object', + objectApiName: 'object', + filters: 'filter', + where: 'filter', + // The date field: `field` is what a record-change trigger calls its + // field, and `dateFieldName`/`targetField` are the same idea one word + // out. None is within edit distance of `dateField`. + field: 'dateField', + dateFieldName: 'dateField', + targetField: 'dateField', + limit: 'maxRecords', + batchSize: 'maxRecords', + }, + guidance: { + // The cadence knob is real, but it is a SIBLING of this descriptor on + // the same `config`, not a member of it — the wrong-layer case the + // guidance channel exists for. `FlowSchema` carries the same + // prescription for a top-level `schedule`. + schedule: + '`schedule` is a sibling of `timeRelative` on the START node\'s `config`, not a key ' + + 'inside it — write `config: { timeRelative: {…}, schedule: { type: \'cron\', ' + + 'expression: \'0 8 * * *\' } }`. Omitting it means daily at 08:00 UTC.', + runAs: + '`runAs` is a FLOW-level key, not part of the descriptor. A sweep has no trigger ' + + 'user, so under the default `runAs: \'user\'` its data operations are REFUSED ' + + '(#3760) — declare `runAs: \'system\'` beside `nodes`/`edges`.', + }, + history: + 'Until #4001 these were dropped silently — the descriptor still parsed and the sweep ' + + 'still bound, so a mis-spelled window or filter produced a trigger that matched ' + + 'nothing (or everything) while reporting itself as configured.', + }, + { /** * Object whose records are swept. Its machine name — the canonical id * everywhere (matches exactly, snake_case). @@ -144,10 +204,10 @@ export const TimeRelativeTriggerSchema = lazySchema(() => .positive() .optional() .describe('Max records launched per sweep (default 1000). The sweep logs when it clamps.'), - }) - .refine((v) => (v.withinDays === undefined) !== (v.offsetDays === undefined), { - message: 'Provide exactly one of `withinDays` (range mode) or `offsetDays` (offset mode).', - }), + }, + ).refine((v) => (v.withinDays === undefined) !== (v.offsetDays === undefined), { + message: 'Provide exactly one of `withinDays` (range mode) or `offsetDays` (offset mode).', + }), ); export type TimeRelativeTrigger = z.infer; diff --git a/packages/spec/src/automation/webhook.test.ts b/packages/spec/src/automation/webhook.test.ts index 086dfa3526..d0d7d1458c 100644 --- a/packages/spec/src/automation/webhook.test.ts +++ b/packages/spec/src/automation/webhook.test.ts @@ -221,3 +221,75 @@ describe('WebhookSchema', () => { })).toThrow(); }); }); + +// #4001 batch 11. The ledger carried this row as `authorable (p)` — "spec-only +// (#3461)". Resolving the `(p)` found the opposite: three parse doors, one of +// them a BOOT path. `plugin-webhooks`' `bootstrapDeclaredWebhooks` re-parses +// every declared webhook before materializing it into `sys_webhook`, and a +// failure there warns and SKIPS the subscription — so a rejection here is the +// difference between a webhook that exists and one that does not. +describe('unknown keys are rejected, not stripped (#4001 batch 11)', () => { + const valid = { name: 'wh_probe', url: 'https://hooks.example/x' }; + const unknownKeyIssue = (value: unknown) => { + const result = WebhookSchema.safeParse(value); + expect(result.success).toBe(false); + return result.error!.issues.find((i) => i.code === 'unrecognized_keys'); + }; + + it('points the sys_webhook COLUMN names back at the spec keys', () => { + // `mapWebhookToRow` (plugin-webhooks) remaps `object → object_name` and + // `isActive → active` on the way into the table. Someone reading a row back + // and re-authoring from those column names is the concrete path here, and + // neither word is within edit distance of the key it means. + expect(unknownKeyIssue({ ...valid, object_name: 'task' })!.message) + .toContain('`object_name` → `object`'); + expect(unknownKeyIssue({ ...valid, active: true })!.message) + .toContain('`active` → `isActive`'); + }); + + it('points `events` at `triggers`', () => { + expect(unknownKeyIssue({ ...valid, events: ['create'] })!.message) + .toContain('`events` → `triggers`'); + }); + + it('carries the #3494 removals as prescriptions, not bare rejections', () => { + for (const key of ['body', 'payloadFields', 'includeSession', 'authentication', 'retryPolicy']) { + const message = unknownKeyIssue({ ...valid, [key]: {} })!.message; + expect(message, `\`${key}\` should carry its #3494 reason`).toContain('#3494'); + } + }); + + // The reason the ADR-0010 envelope is part of this change and not a + // follow-up. Both metadata load paths call `applyProtection` on EVERY type, + // so a package-loaded webhook already carries these keys by the time the boot + // parse sees it. `.strip` discarded them silently; `.strict()` without this + // declaration would have turned every package-shipped webhook into a skipped + // subscription after a redeploy — with one `warn` to say so. + it('accepts the ADR-0010 protection envelope the loader stamps on it', () => { + const parsed = WebhookSchema.parse({ + ...valid, + _packageId: 'com.example.app', _packageVersion: '1.0.0', _provenance: 'package', + _lock: 'no-overlay', _lockSource: 'artifact', _lockReason: 'ships with the package', + }); + expect(parsed._packageId).toBe('com.example.app'); + expect(parsed._provenance).toBe('package'); + }); + + it('accepts an authored `protection` block (the pre-envelope half of ADR-0010)', () => { + expect(WebhookSchema.safeParse({ + ...valid, + protection: { lock: 'full', reason: 'Ships with the package.' }, + }).success).toBe(true); + }); + + it('still accepts every key an author writes', () => { + const parsed = WebhookSchema.parse({ + name: 'showcase_task_changed', label: 'Task Changed', + object: 'showcase_task', triggers: ['create', 'update', 'delete'], + url: 'https://hooks.example/showcase/task', method: 'POST', + headers: { 'X-A': 'b' }, timeoutMs: 5000, secret: 's', isActive: false, + description: 'd', + }); + expect(parsed.isActive).toBe(false); + }); +}); diff --git a/packages/spec/src/automation/webhook.zod.ts b/packages/spec/src/automation/webhook.zod.ts index 8f113355c1..f64df3b14d 100644 --- a/packages/spec/src/automation/webhook.zod.ts +++ b/packages/spec/src/automation/webhook.zod.ts @@ -2,6 +2,9 @@ import { z } from 'zod'; import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; +import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; +import { ProtectionSchema } from '../shared/protection.zod'; +import { strictObject } from '../shared/strict-object'; /** * Webhook Trigger Event @@ -99,11 +102,86 @@ export type WebhookTriggerType = z.infer; * removed too: delivery retries are owned by the messaging outbox's fixed * schedule, which never read the authored policy. The inbound * `WebhookReceiverSchema` (never consumed by any runtime) was removed as well. + * + * ## Closed against unknown keys (#4001 batch 11, ADR-0078) + * + * The ledger carried this row as `authorable (p)` — provisional, "spec-only + * (#3461)". Resolving the `(p)` found the opposite of spec-only: this schema is + * `.parse()`d at THREE doors, one of which is a boot path. + * + * 1. {@link defineWebhook} — the authoring factory. + * 2. `defineStack({ webhooks })` — `StackSchema` holds `z.array(WebhookSchema)`. + * 3. **Boot** — `plugin-webhooks`' `bootstrapDeclaredWebhooks` re-parses every + * declared webhook before materializing it into a `sys_webhook` row + * (`bootstrap-declared-webhooks.ts`), and a parse failure there warns and + * SKIPS the webhook. So an undeclared key here is not merely stripped — from + * this change on it is the difference between a subscription that exists and + * one that does not, which is why the rejection has to carry its fix. + * + * Door 3 is also why the ADR-0010 envelope below is part of this change rather + * than a follow-up. `applyProtection` stamps `_packageId` / `_provenance` (and + * `_lock*` when a `protection` block was authored) onto EVERY metadata item at + * load, including this one — so the object that reaches the boot parse already + * carries keys this schema never declared. They were silently stripped while it + * was `.strip`; closing it without declaring them would have converted every + * package-loaded webhook into a skipped subscription at boot. That is the + * "envelope debt" the campaign paid down eight times over on the registered + * metadata types; `webhook` is not one of those (no + * `BUILTIN_METADATA_TYPE_SCHEMAS` entry), which is precisely why nothing had + * caught it here. */ -export const WebhookSchema = lazySchema(() => z.object({ +export const WebhookSchema = lazySchema(() => strictObject({ + surface: 'this webhook', + aliases: { + // Real, in-repo spellings of the same field on the OTHER side of the + // materialization bridge: `sys_webhook`'s columns are `object_name` / + // `active` (`mapWebhookToRow` in plugin-webhooks does the remap). An admin + // or agent reading a row back and re-authoring it from those column names + // is the concrete path here, and neither word is within edit distance. + object_name: 'object', + objectName: 'object', + active: 'isActive', + enabled: 'isActive', + // The trigger list, named as the neighbouring surfaces name it. + events: 'triggers', + on: 'triggers', + endpoint: 'url', + target: 'url', + signingSecret: 'secret', + }, + guidance: { + // #3494 removed these five and `retryPolicy` — the tombstone has to carry + // the reason, or an author just writes them again one release later. + body: '`body` was removed in 17 (#3494) — the delivery path always sends its own fixed envelope, so a custom body was never sent. There is no replacement.', + payloadFields: '`payloadFields` was removed in 17 (#3494) — the delivery envelope is fixed and was never trimmed to a field list. There is no replacement.', + includeSession: '`includeSession` was removed in 17 (#3494) — session context was never included in a delivery. There is no replacement.', + authentication: '`authentication` was removed in 17 (#3494) — only HMAC signing via `secret` is applied to a delivery. Use `secret`.', + retryPolicy: '`retryPolicy` was removed in 17 (#3494) — delivery retries are owned by the messaging outbox on a fixed schedule, and the authored policy was never read. There is no replacement.', + tags: '`tags` was removed in 17 (#3494) — nothing read them. There is no replacement.', + }, + history: + 'Until #4001 these were dropped silently — the webhook still parsed and still ' + + 'materialized, so a subscription scoped or secured with a key we do not declare ' + + 'shipped listening to the wrong thing, or to everything.', + // `WebhookConfigSchema` (integration/connector.zod.ts) is this shape + // `.extend()`ed, and zod carries BOTH the strictness and this error map onto + // the extension — verified against real zod, not assumed. Naming the + // extension's own key keeps a typo of it fixable on that surface. + // + // Its SIBLING key `events` is deliberately NOT listed here even though the + // extension declares it, because it is an alias target above: listing it + // would make a base-surface typo suggest `events`, and writing `events` would + // then suggest `triggers` — an author walked through two rejections to a key + // the base does not accept. That is finding 7 (the `triggerPhrase` → + // `triggerPhrases` → tombstone chain) arriving from a new direction: + // `acceptsNothing` guards the shape-derived candidates, and nothing guards + // hand-written `extraKeys`. On the connector surface `events` is declared, so + // it is never the unrecognized key there anyway. + extraKeys: ['signatureAlgorithm'], +}, { name: SnakeCaseIdentifierSchema.describe('Webhook unique name (lowercase snake_case)'), label: z.string().optional().describe('Human-readable webhook label'), - + /** Scope */ object: z.string().optional().describe('Object whose record events (create/update/delete, bulk_update/bulk_delete) trigger this webhook'), triggers: z.array(WebhookTriggerType).optional().describe('Events that trigger execution'), @@ -126,6 +204,27 @@ export const WebhookSchema = lazySchema(() => z.object({ /** Metadata */ description: z.string().optional().describe('Webhook description'), + + /** + * ADR-0010 §3.7 — Package-level protection envelope. Package authors declare + * lock policy here; the loader translates it into the private `_lock` + * envelope at registration time and strips this block before persistence. + * See `shared/protection.zod.ts`. + */ + protection: ProtectionSchema.optional().describe( + 'Package author protection block — lock policy for this webhook.', + ), + + // ADR-0010 — runtime protection envelope (internal — set by loader). + // + // Declared in #4001 batch 11 for the reason the module header states: both + // metadata load paths call `applyProtection` on EVERY type, so a + // package-loaded webhook has always carried these keys by the time + // `bootstrapDeclaredWebhooks` re-parses it. `.strip` discarded them; the + // `.strict()` gate above would have rejected them, and the observable failure + // would not have looked like a validation problem at all — just a webhook + // that stopped existing after a redeploy, with one `warn` to say so. + ...MetadataProtectionFields, })); export type Webhook = z.infer; diff --git a/packages/spec/src/integration/connector.test.ts b/packages/spec/src/integration/connector.test.ts index c9c59d633c..852580556c 100644 --- a/packages/spec/src/integration/connector.test.ts +++ b/packages/spec/src/integration/connector.test.ts @@ -274,6 +274,28 @@ describe('WebhookConfigSchema', () => { expect(parsed.signatureAlgorithm).toBe('hmac_sha256'); expect(parsed.timeoutMs).toBe(30000); }); + + // #4001 batch 11 closed the BASE (`automation/webhook.zod.ts`), and zod + // carries both the strictness and the base's error map through `.extend()`. + // That is the trap the ledger records as finding 16 — a base tightened for + // one surface silently retightening another — so it is asserted here, on the + // extension's own file, rather than left for someone to discover. + it('inherits the base webhook\'s strictness through `.extend()` (#4001)', () => { + const result = WebhookConfigSchema.safeParse({ + name: 'test_webhook', url: 'https://api.example.com/webhooks', notAKey: 1, + }); + expect(result.success).toBe(false); + expect(result.error!.issues.some((i) => i.code === 'unrecognized_keys')).toBe(true); + }); + + it('still accepts the two keys the extension adds', () => { + // The base names `signatureAlgorithm` in `extraKeys` so a typo of it is + // still suggestible on this surface, where the base has never heard of it. + expect(WebhookConfigSchema.safeParse({ + name: 'test_webhook', url: 'https://api.example.com/webhooks', + events: ['record.created'], signatureAlgorithm: 'hmac_sha512', + }).success).toBe(true); + }); }); // ============================================================================