From 58a511a5fb16139d1e5ed55e28cd6a6e8b082325 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 17:29:07 +0000 Subject: [PATCH 1/3] feat(spec)!: close nine authorable automation shapes against unknown keys (#4001 batch 11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit zod's default `.strip` discards an undeclared key and parses on. Batch 11 closes the nine remaining shapes in `automation/`'s main body, each rejection naming the surface, the key, and — where the word is recognisable — the canonical spelling. `flow.zod.ts`: the four OUTER shapes were closed earlier; their six nested authoring blocks were not, so the gate rejected `nodee:` at node level while `connectorConfig: { connectorID }` parsed clean and dispatched against an undefined connector id. Now strict: `FlowNode.connectorConfig` / `.position` / `.inputSchema` / `.waitEventConfig` / `.boundaryConfig`, `Flow.errorHandling`. Deliberately still open, now pinned in code AND tests so a later sweep stops rather than "finishing" the file: the node `config` slot (ADR-0018) and `FlowVersionHistorySchema` (emitted, not authored — its `definition` is `FlowSchema`, so the authored half is gated anyway). Every alias is a real in-repo spelling of the same knob on a neighbouring surface, taken from an AST census over the repo's own payloads, not guessed: `backoffMs` from `shared/retry-policy.zod.ts` (#4661), `initialDelayMs` / `maxDelayMs` from connector `RetryConfig`, `attachedToRef` / `cancelActivity` from BPMN 2.0. Two cases get a prescription instead of a rename because a rename would be wrong: `inputSchema.optional` is the opposite polarity of `required`, and `errorHandling.maxAttempts` counts the first attempt while `maxRetries` does not — renaming it alone would quietly run one attempt fewer. `time-relative-trigger.zod.ts`: the schema #4852 made visible (its only site was line-wrapped, the old counter read zero, and a zero-site file is skipped by the coverage walk). It sits under the open node `config` slot, so it is the only key gate there is — and it is `safeParse`d at BIND time, so `offsetDay` beside a valid mode used to bind a sweep with the author's narrowing discarded and report itself configured. `flow-function.zod.ts`: binds at authoring only (boot reads `normalizeFlowFunctionEntry`), stated in the code rather than implied. That is why it matters: the reader takes two keys and ignores the rest by construction, so a misspelled `effect` was dropped and then not looked for — the function ran and its writes were counted as none, keeping #4354's broken-sweep query silent. `webhook.zod.ts`: the ledger's `(p)` resolved to the opposite of "spec-only" — three parse doors, one of them `bootstrapDeclaredWebhooks` at BOOT, which warns and SKIPS on failure. Hence the ADR-0010 envelope in the same commit: `applyProtection` stamps `_packageId`/`_provenance` on every type, so closing the shape without declaring them would have turned every package-shipped webhook into a skipped subscription after a redeploy. Strictness also rides `.extend()` onto the connector `WebhookConfigSchema` — verified against real zod and pinned in `connector.test.ts`. Ledger: `automation/` 67 → 58 strip; three rows reached zero and were deleted by the #4852 reverse pin. Gate re-proved red in both directions before trusting it, and every new assertion was proved to depend on the tightening by neutering `strictObject` (19 red, restored byte-identical). Part of #4001 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --- ...known-key-strictness-automation-batch11.md | 76 +++++++ content/docs/automation/flows.mdx | 2 +- .../2026-07-unknown-key-strictness-ledger.md | 17 +- packages/spec/authorable-surface.json | 16 ++ .../spec/src/automation/flow-function.test.ts | 61 +++++ .../spec/src/automation/flow-function.zod.ts | 53 ++++- packages/spec/src/automation/flow.test.ts | 149 +++++++++++++ packages/spec/src/automation/flow.zod.ts | 209 ++++++++++++++++-- .../automation/time-relative-trigger.test.ts | 65 ++++++ .../automation/time-relative-trigger.zod.ts | 72 +++++- packages/spec/src/automation/webhook.test.ts | 72 ++++++ packages/spec/src/automation/webhook.zod.ts | 103 ++++++++- .../spec/src/integration/connector.test.ts | 22 ++ 13 files changed, 882 insertions(+), 35 deletions(-) create mode 100644 .changeset/unknown-key-strictness-automation-batch11.md 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/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 1cc370872e..b1568cf312 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -527,7 +527,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 }` — one capital letter — parsed clean and dispatched against an undefined connector id. 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 (p) | **−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 | @@ -538,9 +538,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; candidate once the executors' own drift is verified | | `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. Same candidacy note as `io-node-config` | | `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). Since #4343 `script` and `subflow` ARE parsed at execute time (`parse-config.ts`) — `script` once retiring its `actionType` branches left it flat — so strictness candidacy now follows `io-node-config` on the same terms rather than being moot; `decision` stays export-only | -| `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. @@ -605,25 +605,22 @@ 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/` — 67 strip of 75 +#### `automation/` — 58 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) | | `builtin-node-config.zod.ts` | 8 | 8 | authorable | CRUD quartet + `Screen` (+ `.options`) + `Map`; already has a bidirectional drift check (`builtin-node-form-zod-ledger.test.ts`) | -| `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 | | `state-machine.zod.ts` | 6 | 6 | authorable (p) | `ActionRef` / `GuardRef` / `Transition` / `StateNode` + `.meta` / `StateMachine` | | `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 | | `control-flow.zod.ts` | 5 | 5 | authorable (p) | `FlowRegion` / `Loop` / `ParallelBranch` / `Parallel` / `TryCatch` — validated structurally by `validateControlFlow` today, which is a sibling guard, not a key gate | | `node-executor.zod.ts` | 4 | 4 | wire | **out of scope** — executor registration contract, code-to-code | | `schemaless-node-config.zod.ts` | 4 | 4 | authorable | `Script` / `Subflow` / `DecisionCondition` / `Decision`; `script` + `subflow` ARE parsed at execute time since #4343 | | `io-node-config.zod.ts` | 2 | 2 | authorable | `NotifyConfig` / `HttpConfig` — the sibling contracts for the deliberately-open flow node `config` slot | -| `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) | -**Authorable strip in `automation/`: 41 of 67.** This is the ruling's "known main body". +**Authorable strip in `automation/`: 32 of 58.** This is the ruling's "known main body"; batch 11 took 9 of it (the six nested `flow.zod.ts` blocks + `time-relative-trigger` + `flow-function` + `webhook`), and three files reached zero strip sites, so their rows are gone — the reverse pin above did the deleting, not a memory. #### `ui/` — 123 strip of 198 diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index e1e005ce6b..26d64fb50c 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", @@ -4175,6 +4183,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", @@ -4183,6 +4198,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..8a146cf95f 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -1306,4 +1306,153 @@ 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`, so the gate rejected `nodee:` at the node level + // while `connectorConfig: { connectorID }` — one capital letter — parsed + // clean and dispatched against an undefined connector id. Same defect, one + // layer in: a guard put where the author who wrote it was standing. + 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`'); + // The one that motivates the block: a case slip on the id the executor + // reads, which used to dispatch with `connectorId: undefined`. + expect(unknownKeyIssue(FlowNodeSchema, node({ + connectorConfig: { connectorID: 'rest', actionId: 'get' }, + }))!.message).toContain('`connectorID` → `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..9b7f2339ed 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,22 @@ 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 }` — one + * capital letter — still parsed clean and dispatched the action against an + * undefined connector id. 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 +191,79 @@ 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 ' + + 'mis-spelled connector/action id or a whole input map was ignored and the ' + + 'action dispatched with `input ?? {}` against `undefined`.', + }, + { + 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 node ' + + 'carrying a size or a third coordinate landed on the canvas at (0, 0) instead.', + }, + { 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 the author marked required was generated as an optional Studio form ' + + 'field and never validated at run time.', + }, + { + 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 +279,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 +351,27 @@ 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 a boundary ' + + 'event whose host node the author named with the BPMN word attached to nothing and ' + + 'never fired.', + }, { /** 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 +605,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 +742,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 be16927e8d..65977fc28b 100644 --- a/packages/spec/src/integration/connector.test.ts +++ b/packages/spec/src/integration/connector.test.ts @@ -275,6 +275,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); + }); }); // ============================================================================ From 1114d54ed263daeaf33413d6c87945958ba44e37 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 17:33:47 +0000 Subject: [PATCH 2/3] docs(spec): regenerate reference pages for the webhook ADR-0010 envelope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gen:docs` only — the property tables for `automation/Webhook` and the `integration/WebhookConfig` extension gain the `protection` block and the seven `_lock*`/`_provenance`/`_package*` rows. Checked for the #3746 trap: no page description or frontmatter moved, so no internal comment leaked onto a public reference page. Part of #4001 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --- content/docs/references/automation/webhook.mdx | 8 ++++++++ content/docs/references/integration/connector.mdx | 8 ++++++++ 2 files changed, 16 insertions(+) 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 89b4fef6d5..0298045ccf 100644 --- a/content/docs/references/integration/connector.mdx +++ b/content/docs/references/integration/connector.mdx @@ -517,6 +517,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 | From de2979df6034a6b40b1804cfeb2b4e6523b5456e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 17:49:23 +0000 Subject: [PATCH 3/3] fix(spec): correct three history strings that named the wrong failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review against the actual behaviour, before the claim shipped. The `connectorConfig` / `boundaryConfig` / `position` / `inputSchema` prose named a slip on a REQUIRED key (`connectorID`, `attachedToRef`) as the silent case. It never was: a required key spelled wrong then reads as MISSING, which zod has always rejected loudly. Nothing was silent about it. What `.strip` actually swallowed is the OPTIONAL half — the mapped `input` map (dispatched as `input ?? {}`: a successful connector call carrying nothing), BPMN's `cancelActivity: false` (so `interrupting` fell to its `true` default and a NON-interrupting event cancelled the host anyway), `optional: false` (so `required` fell to false and `validateNodeInputSchemas` had nothing to require), a canvas hint beside x/y. I.e. exactly the keys an author adds to CONSTRAIN behaviour, replaced by a permissive default. The corrected version is the stronger argument as well as the true one, and getting it wrong is the failure mode this campaign has already paid for four times (the withdrawn guidance prescriptions): rejection prose is behaviour, not documentation. Fixed in the schemas, the module comment, the tests and the ledger row; a test now pins the required-key case as loud so the distinction cannot quietly re-blur. Part of #4001 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --- .../2026-07-unknown-key-strictness-ledger.md | 2 +- packages/spec/src/automation/flow.test.ts | 28 ++++++++---- packages/spec/src/automation/flow.zod.ts | 44 ++++++++++++------- 3 files changed, 50 insertions(+), 24 deletions(-) diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index b1568cf312..9ea77ac149 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -527,7 +527,7 @@ not verdicts). | File | Sites | Class | Note | |---|---|---|---| -| `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 }` — one capital letter — parsed clean and dispatched against an undefined connector id. 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) | +| `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 (p) | **−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 | diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 8a146cf95f..c3a0e3bf17 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -1310,10 +1310,15 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { // ── batch 11: the INNER blocks ──────────────────────────────────────────── // // Closing the four outer shells above left six nested authoring blocks on - // zod's default `.strip`, so the gate rejected `nodee:` at the node level - // while `connectorConfig: { connectorID }` — one capital letter — parsed - // clean and dispatched against an undefined connector id. Same defect, one - // layer in: a guard put where the author who wrote it was standing. + // 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 }); @@ -1323,11 +1328,18 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { })); expect(issue!.message).toContain("connector_action node's `connectorConfig`"); expect(issue!.message).toContain('`params` → `input`'); - // The one that motivates the block: a case slip on the id the executor - // reads, which used to dispatch with `connectorId: undefined`. - expect(unknownKeyIssue(FlowNodeSchema, node({ + }); + + 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' }, - }))!.message).toContain('`connectorID` → `connectorId`'); + })); + 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)', () => { diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 9b7f2339ed..fdd43409a6 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -85,11 +85,21 @@ export const FLOW_STRUCTURAL_NODE_TYPES: readonly string[] = ['start', 'end']; * / `.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 }` — one - * capital letter — still parsed clean and dispatched the action against an - * undefined connector id. 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). + * `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 @@ -206,9 +216,9 @@ export const FlowNodeSchema = lazySchema(() => z.object({ payload: 'input', }, history: - 'Until #4001 these were dropped silently — the block still parsed, so a ' + - 'mis-spelled connector/action id or a whole input map was ignored and the ' + - 'action dispatched with `input ?? {}` against `undefined`.', + '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'), @@ -229,8 +239,10 @@ export const FlowNodeSchema = lazySchema(() => z.object({ { surface: "this node's canvas `position`", history: - 'Until #4001 these were dropped silently — the block still parsed, so a node ' + - 'carrying a size or a third coordinate landed on the canvas at (0, 0) instead.', + '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(), @@ -255,8 +267,9 @@ export const FlowNodeSchema = lazySchema(() => z.object({ }, history: 'Until #4001 these were dropped silently — the declaration still parsed, so a ' + - 'parameter the author marked required was generated as an optional Studio form ' + - 'field and never validated at run time.', + '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'), @@ -368,9 +381,10 @@ export const FlowNodeSchema = lazySchema(() => z.object({ duration: 'timerDuration', }, history: - 'Until #4001 these were dropped silently — the block still parsed, so a boundary ' + - 'event whose host node the author named with the BPMN word attached to nothing and ' + - 'never fired.', + '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'),