diff --git a/.changeset/unknown-key-strictness-automation-etl.md b/.changeset/unknown-key-strictness-automation-etl.md new file mode 100644 index 0000000000..d0b5eef87b --- /dev/null +++ b/.changeset/unknown-key-strictness-automation-etl.md @@ -0,0 +1,61 @@ +--- +"@objectstack/spec": major +--- + +feat(spec)!: reject unknown keys on the ETL authoring contracts (#4001 批 12) + +The final `automation/` wave of the 2026-08-03 "necessary-and-complete" ruling. +Seven strip sites in `automation/etl.zod.ts` close, and `automation/`'s +remaining-strip count drops 53 → 46 (authorable 27 → 20). + +Now strict: `ETLSourceSchema` (and its `incremental` block), +`ETLDestinationSchema`, `ETLTransformationSchema`, `ETLPipelineSchema` (and its +`retry` and `notifications` blocks). + +**Deliberately still open: `ETLPipelineRunSchema`, `.stats` and `.error`.** +Every key on those is a fact the engine produces about a run that already +happened — an id it minted, a status it reached, counters it accumulated. +Nobody authors a run result, so strictness buys no author protection there, +while it would turn a future engine reporting one more counter into a parse +crash for every existing reader. Same disposition, same reason, as +`FlowVersionHistorySchema` and all of `execution.zod.ts`. + +**Migration.** Every key now rejected was previously stripped and had no +runtime effect, so removing or renaming one never changes behaviour. No +ADR-0087 conversion is needed: the three shipped example apps' built artifacts +were walked (3930 nodes) with 0 shapes newly rejected, the probe proven red +first on an injected control, and all three `objectstack validate` runs pass. + +The rejections carry their own prescriptions: + +- **source / destination / transformation**: the common mistake here is not a + typo but a MISPLACEMENT. `table`, `schema`, `endpoint`, `path`, `format`, + `condition`, `groupBy` are real settings that belong one level down, inside + the open `config` record; every message on these three surfaces says so. +- `source.incremental`: `timestampField` → `cursorField` (the connector layer's + `DataSyncConfig` spells the same thing `timestampField`). +- `destination`: `strategy` → `writeMode`; on the **pipeline** the same word is + `strategy` → `syncMode`. The connector's one `strategy` enum + (`full | incremental | upsert | append_only`) splits across those two keys — + the write half on the destination, the extraction half on the pipeline. +- pipeline `direction`: not a key at all, by design. An ETL pipeline states + direction structurally, by which endpoint is `source` and which is + `destination`; to reverse one, swap the two endpoints. +- `retry`: `maxRetries` → `maxAttempts`, and `retryDelayMs` → `backoffMs` (the + pre-17 spelling retired in #4661). `backoffMultiplier`, `maxRetryDelayMs` and + `jitter` are declared on the converged `RetryPolicySchema` and **deliberately + absent** here — a documented absence, not a typo. Converging the two retry + vocabularies is tracked as #4962. +- `notifications`: `onError` → `onFailure`. + +**One published-artifact change, not a no-op.** The campaign's standing claim +that strictness does not move the published JSON Schema holds per direction: +`build-schemas.ts` prefers `io: 'output'`, where a stripping object already +emits `additionalProperties: false`. `ETLPipelineSchema` is the case where that +does not apply — it cannot convert in output mode at all (`schedule` is +`CronExpressionInputSchema`, a transform), so the build falls back to input +mode, and there strip emits nothing while strict emits +`additionalProperties: false`. The published pipeline schema therefore narrows +from "unspecified" to "closed", which is the intended direction — the +publication now matches the parse instead of being quieter than it. +`ETLPipelineRun` publishes under output mode and did not move. diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 929cf548cd..9ead54a487 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -529,7 +529,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, 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 | +| `etl.zod.ts` | 10 | mixed | **7 strict as of #4001 批 12** — the authoring half (`ETLSource` + `.incremental`, `ETLDestination`, `ETLTransformation`, `ETLPipeline` + `.retry` + `.notifications`). The other 3 — `ETLPipelineRun` + `.stats` + `.error` — are **deliberately left open**: engine-emitted run state (an id it minted, a status it reached, counters it accumulated), same disposition and same reason as `FlowVersionHistorySchema` above and all of `execution.zod.ts`. The exemption is recorded on the schema itself, not only here, because a note only this file carries is a note the next sweep does not read. The old blanket `authorable (p)` was too wide; verification split it. ⚠️ **Read the classification caveat before reusing this verdict**: `etl.zod.ts` has NO parse site in objectstack / objectui / cloud, so neither half could be settled by pointing at a live call. The 7 are authorable because the exported schema and type ARE the door (`SYNC_ARCHITECTURE.md` and the module's `@example` both hand-write `const p: ETLPipeline = { … }`) — the `webhook.zod.ts` posture. The 3 are wire on the shape's semantics plus settled precedent, NOT on an emit site anyone can point at today; if an ETL engine ever lands and a run result turns out to be operator-authored, that verdict is the one to revisit. Two out-of-scope findings were filed rather than fixed here: the `retry` block is a third retry-policy vocabulary #4661's convergence never reached (#4962), and all nine type aliases export the parsed shape under the bare name, which is why the SYNC_ARCHITECTURE.md pipeline examples do not compile (#4963). **−12 at #4738**: `sync.zod.ts` (the L1 "Simple Sync" file — `DataSyncConfig`, its `ConflictResolution` enum and satellites, formerly this row's co-candidate) was deleted whole rather than hardened: three-repo zero importers, no parse site, defs unreachable from the metadata-type roots (#4650 gate), so there was no author for strictness to protect (#4535 C13+C15). The integration-side `ConflictResolution` → `ConnectorConflictResolution` rename in the same change is name-only and moves no sites | | `execution.zod.ts` | 13 | wire | run-state envelopes — never strict. +5 at #4354 (the run-summary family: step metrics / skip reason / per-node / per-gate / the summary itself) — engine-emitted telemetry read by the Console and by operator queries, nobody authors them, so the `wire` verdict covers them unchanged | | `state-machine.zod.ts` | 6 | authorable | **strict as of #4001 批 10** — all six sites (`ActionRef` / `GuardRef` / `Transition` / `StateNode` + `.meta` / `StateMachine`). **The `(p)` was NOT a formality here.** ADR-0020 retired this XState shape as a *record-lifecycle* declaration — the top-level `workflow` metadata type and `object.stateMachines` are both gone, and a record's transitions live on the `state_machine` VALIDATION RULE instead — so had those been the only doors this file would be DEAD surface, and the correct action would have been to fix its class, not close it. One authoring door survives: `ai/agent.zod.ts`'s `lifecycle` is `StateMachineSchema`, and `agent` is a registered type, so `defineStack({ agents })` / meta REST / the Studio agent form all reach here through `AgentSchema.parse()`. Verified by parse: an agent whose lifecycle carried `stats`, a state with `onn` (one keystroke from `on`) and a `meta` with two unknown keys **parsed clean**, returning a machine with NO transitions at all — the declaration whose whole job is to deny undeclared transitions, silently emptied and reported valid. `.meta` was checked for the #4909 open-slot case and is CLOSED: the hand-written `StateNodeConfig` type declares exactly its four keys (passthrough would open the Zod while `tsc` stayed shut), nothing in the repo reads any `meta` key, and the prior behaviour was strip — an author's `meta` arrived as `{}` — so there was no openness to preserve. ⚠️ `ActionRef` / `GuardRef` are UNIONS: a strict branch's message does not reach the top (zod raises one `invalid_union` whose message is the literal `"Invalid input"`, with the real prescription nested in `issue.errors[]`), which `formatZodError` then flattens away — filed, not fixed here. **−1 at #4658**: the orphan `EventSchema` (`{ type, schema }`, an XState-style signal declaration nothing referenced — `StateMachineSchema` names event types as `on:` record keys) was deleted rather than converged with `kernel/events/core.zod.ts`'s envelope `EventSchema`, whose key set it did not intersect (#4535 C6). The remaining 6 sites and their verdict are unchanged | | `control-flow.zod.ts` | 5 | authorable | **strict as of #4001 批 10** — all five sites (`FlowRegion` / `Loop` / `ParallelBranch` / `Parallel` / `TryCatch`). The `(p)` resolves to authorable on the executors' own parse seam (`parseNodeConfig`, #4277) plus `validateControlFlow`'s region parse. **`validateControlFlow` is a sibling guard, not a key gate, and the two do not fight**: it answers single-entry / single-exit / acyclic, which no key check can decide, and the schema answers key membership, which no structural check can decide. They meet at exactly one seam — the guard `safeParse`s each region slot before analyzing it, so an undeclared region key now surfaces there as `: invalid region — `, the guard's framing wrapping the schema's prescription. Nothing was duplicated and nothing removed; the guard simply stopped silently repairing its own input before judging it. Two curation entries had to be MEASURED rather than reasoned: the bare edit-distance fallback answers `itemVariable` with **`indexVariable`** — binding the loop INDEX where the author wanted the ITEM — so the alias exists to overrule a confidently wrong suggestion from this campaign's own helper (the `pii` → `min` shape, third instance); and `join`/`joinGateway` needed two DISTINCT prescriptions because `guidance` emits one bullet per key verbatim, so a shared string printed the same paragraph twice. Its test instrument also had to be rebuilt: `region-slots.test.ts` probed every construct with every candidate key at once and depended on `.strip` to discard the mismatches, so it returned "no schema accepts any region" the moment the shapes closed — it failed loudly, which is the only reason this is a footnote and not a fourth finding-3. Structural validation by `validateControlFlow` remains. **−1 at #4661**: `RetryPolicySchema` moved out to `shared/retry-policy.zod.ts` — `./automation` and `./system` published the same name for two different declarations (#4411), so the retry policy converged onto one. The site still exists and is still non-strict and authorable; it is simply no longer in a directory this ledger sections. ⚠️ That is a coverage gap worth knowing about: this audit sections `ui/` / `data/` / `automation/` / `security/` / `studio/` only, so a `shared/` shape is unaudited by construction. The tolerance is deliberate here — the `retryDelayMs` → `backoffMs` rename is tombstoned via `retiredKey()` precisely because a non-strict parent would otherwise swallow the old spelling | @@ -606,12 +606,12 @@ 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/` — 33 strip of 75 +#### `automation/` — 26 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) | +| `etl.zod.ts` | 3 | 10 | wire | **Authorable half closed at 批 12** (7 sites: `ETLSource` + `.incremental`, `ETLDestination`, `ETLTransformation`, `ETLPipeline` + `.retry` + `.notifications`). What is left is `ETLPipelineRun` + `.stats` + `.error` — engine-emitted run state, exempt for the `FlowVersionHistorySchema` reason and pinned as such in `etl.test.ts`, so closing it means deleting a test that says not to. **This row shrinks without disappearing** — the second in `automation/` to do so, after `flow.zod.ts` reached its own wire floor of 1 at 批 11 (the two batches were in flight together and arrived at the same shape independently, which is the better evidence that it is the right one). Worth naming because the reverse pin cannot see it: the pin fires on zero, so a row that stops at its wire floor looks exactly like a row nobody finished. The Class column is the only thing separating them — read it before treating this as unfinished work | | `flow.zod.ts` | 1 | 11 | wire | **batch 11 closed the 6 authorable** (`FlowNode.connectorConfig` / `.position` / `.inputSchema` / `.waitEventConfig` / `.boundaryConfig`, `Flow.errorHandling`). The 1 left is `FlowVersionHistorySchema`, which this table has exempted since it was written — **do not close it**: it is emitted on publish, not authored, so closing it makes a future emitter-side field a parse failure for whoever reads history. The exemption now also lives beside the schema and in `flow.test.ts`, because a row in a table is not where the next person to open that file will look | | `bpmn-interop.zod.ts` | 5 | 5 | wire (p) | **out of scope** — third-party BPMN import/export shapes; strictness turns an upstream addition into our parse crash | | `node-executor.zod.ts` | 4 | 4 | wire | **out of scope** — executor registration contract, code-to-code | @@ -625,6 +625,7 @@ gate went red on it still being there, not because someone remembered: | **批 9** (#4925) | `builtin-node-config` (8) · `schemaless-node-config` (4) · `io-node-config` (2) | — | | **批 10** (#4973) | `control-flow` (5) · `state-machine` (6) | — | | **批 11** (#4974) | `flow-function` (1) · `time-relative-trigger` (1) · `webhook` (1) | `flow.zod.ts` 7 → 1 | +| **批 12** (#4979) | — | `etl.zod.ts` 10 → 3 | **How those waves met is worth recording, because it is the failure mode this table is most exposed to.** Each PR deleted its own rows and decremented this @@ -633,23 +634,35 @@ overlap — and left only the header conflicted, while the subtotal line below i which conflicts with nothing, **merged clean and wrong**. No number was a mistake in isolation: each was correct against the branch that computed it. -It has now happened three times in one day. 批 10 recorded it against 批 9's +It has now happened four times in one day. 批 10 recorded it against 批 9's header; 批 11 then merged and its subtotal (`etl` 7 + `state-machine` 6 + `control-flow` 5) and 批 10's (`etl` 7 + `flow` 6 + three singles) were each -right against their own branch and both wrong against the merge. So the rule is -mechanical rather than remembered: **the header and the subtotal are recomputed -from the surviving rows, never resolved in favour of a side**, and -`check:strictness-ledger`'s arithmetic is what settles it. A clean-looking merge -here is evidence of nothing. - -**Authorable strip in `automation/`: 7 of 33** (was 41 of 67 before the three -waves). What is left of the ruling's "known main body" is **`etl.zod.ts` alone** -— `ETLSource` + `.incremental`, `ETLDestination`, `ETLTransformation`, -`ETLPipeline` + `.retry` + `.notifications`. The other 26 strip sites here are -wire and out of the ruling's forced scope: `execution` 13, `bpmn-interop` 5, -`node-executor` 4, `etl`'s own 3 run-state shapes, and `flow.zod.ts`'s last site -`FlowVersionHistorySchema` — which is why `flow` still has a row while having 0 -authorable left, and must not be read as unfinished work. +right against their own branch and both wrong against the merge; 批 12 made it +four, and did so twice — once against 批 10 and again against 批 11 — which is +the useful detail, because it means the count is not "once per wave" but once +per *pair* of waves that overlap in flight. So the rule is mechanical rather +than remembered: **the header and the subtotal are recomputed from the surviving +rows, never resolved in favour of a side**, and `check:strictness-ledger`'s +arithmetic is what settles it. A clean-looking merge here is evidence of nothing. + +**Authorable strip in `automation/`: 0 of 26** (was 41 of 67 when the ruling was +written). **The ruling's `automation/` main body is complete** — every remaining +strip site in this directory is wire, and none is in the forced scope: +`execution` 13, `bpmn-interop` 5, `node-executor` 4, `etl`'s 3 run-state shapes, +and `flow.zod.ts`'s last site `FlowVersionHistorySchema`. + +That leaves the section in a state this table has not been in before, and it is +the state most likely to be misread: **two rows now sit at a deliberate wire +floor** — `flow` at 1 (批 11) and `etl` at 3 (批 12) — rather than having +disappeared. The reverse pin cannot see the difference. It fires when a file +reaches zero, so it proves a row's work is *done*; it is completely silent about +a row whose work is *deliberately partial*, and to the gate "finished, the rest +is wire by decision" and "nobody got to it" are the same row. Only the `Class` +column separates them, which makes that column load-bearing from here on rather +than descriptive. Both waves drew the same conclusion independently and acted on +it the same way: the decision is also written beside the schema and pinned in a +test (`flow.test.ts`, `etl.test.ts`), because a row in a table is not where the +next person to open that file will look. #### `ui/` — 123 strip of 198 diff --git a/packages/spec/src/automation/etl.test.ts b/packages/spec/src/automation/etl.test.ts index 56626f7973..f42423d628 100644 --- a/packages/spec/src/automation/etl.test.ts +++ b/packages/spec/src/automation/etl.test.ts @@ -306,3 +306,277 @@ describe('ETL factory', () => { expect(() => ETLPipelineSchema.parse(pipeline)).not.toThrow(); }); }); + +// ─── #4001 批 12 — unknown-key strictness (ADR-0078) ────────────────── +// +// The file splits 7 authorable / 3 wire. Everything below is written to fail +// LOUDLY if either half moves: the seven must reject, the three must tolerate, +// and every rejection is paired with a positive control parsing the SAME +// document minus the offending key — so a red assertion can never be a document +// that was invalid for an unrelated reason (the campaign's "prove the +// instrument red before trusting green"). + +/** A pipeline that parses clean — the base every negative case is built from. */ +const VALID_PIPELINE = { + name: 'customer_360_pipeline', + label: 'Customer 360', + source: { + type: 'api', + connector: 'salesforce', + config: { object: 'Account' }, + incremental: { enabled: true, cursorField: 'updated_at' }, + }, + destination: { + type: 'warehouse', + connector: 'snowflake', + config: { table: 'customers' }, + writeMode: 'upsert', + primaryKey: ['customer_id'], + }, + transformations: [{ name: 'only_active', type: 'filter', config: { condition: 'active' } }], + syncMode: 'incremental', + schedule: '0 2 * * *', + enabled: true, + retry: { maxAttempts: 5, backoffMs: 120000 }, + notifications: { onSuccess: ['data@example.com'], onFailure: ['ops@example.com'] }, + tags: ['analytics'], + metadata: { owner: 'data-team' }, +} as const; + +/** A run result that parses clean — the base for the wire-half tolerance pins. */ +const VALID_RUN = { + id: 'run-001', + pipelineName: 'customer_360_pipeline', + status: 'succeeded', + startedAt: '2024-01-01T02:00:00Z', + completedAt: '2024-01-01T02:15:00Z', + durationMs: 900000, + stats: { recordsRead: 10, recordsWritten: 10, recordsErrored: 0, bytesProcessed: 2048 }, + error: { message: 'none', code: 'OK' }, + logs: ['ok'], +} as const; + +/** Deep-clone the base and drop an unknown key into one nested block. */ +function pipelineWith(path: readonly string[], key: string, value: unknown) { + const doc = structuredClone(VALID_PIPELINE) as Record; + let cursor: Record = doc; + for (const step of path) cursor = cursor[step] as Record; + cursor[key] = value; + return doc; +} + +/** The unknown-key message for `key` written at `path`, or `null` if it parsed. */ +function rejectionFor(path: readonly string[], key: string): string | null { + const result = ETLPipelineSchema.safeParse(pipelineWith(path, key, 'x')); + return result.success ? null : result.error.issues.map((i) => i.message).join('\n'); +} + +describe('[#4001 批 12] the seven authorable shapes reject unknown keys', () => { + it('parses the base document — the positive control every negative below relies on', () => { + const result = ETLPipelineSchema.safeParse(structuredClone(VALID_PIPELINE)); + expect(result.success, result.success ? '' : JSON.stringify(result.error.issues)).toBe(true); + }); + + // Each row: where the key is written, and a distinctive fragment of the + // surface name that must appear in the rejection. The surface name is what + // tells an author WHICH of the seven nested shapes they got wrong, which is + // the difference between a fixable error and a puzzle. + const surfaces: ReadonlyArray = [ + ['pipeline', [], 'this ETL pipeline'], + ['source', ['source'], 'this ETL source'], + ['source.incremental', ['source', 'incremental'], 'this ETL incremental extraction config'], + ['destination', ['destination'], 'this ETL destination'], + ['pipeline.retry', ['retry'], "this ETL pipeline's retry configuration"], + ['pipeline.notifications', ['notifications'], "this ETL pipeline's notification settings"], + ]; + + it.each(surfaces)('rejects an unknown key on %s, naming the surface', (_label, path, surface) => { + const message = rejectionFor(path, 'totallyMadeUpKey'); + expect(message, 'the key must be REJECTED, not stripped').not.toBeNull(); + expect(message).toContain(surface); + expect(message).toContain('`totallyMadeUpKey`'); + // History: every message says what used to happen silently. + expect(message).toContain('Until #4001'); + }); + + it('rejects an unknown key on a transformation — the seventh shape, reached through the array', () => { + const doc = structuredClone(VALID_PIPELINE) as Record; + (doc.transformations as Array>)[0].continueOnFailure = true; + const result = ETLPipelineSchema.safeParse(doc); + expect(result.success).toBe(false); + const message = result.success ? '' : result.error.issues.map((i) => i.message).join('\n'); + expect(message).toContain('this ETL transformation'); + expect(message).toContain('`continueOnFailure`'); + }); + + it('points a misplaced endpoint setting at the open `config` bag', () => { + // The dominant failure on this file is not a typo: `table` IS a real + // setting, one level down. `.strip` deleted it where it stood and the + // pipeline loaded into whatever `config.table` said instead. + const message = rejectionFor(['destination'], 'table'); + expect(message).toContain('inside the open `config` record'); + }); + + it('rejects each of the seven when parsed standalone, not only through the pipeline', () => { + // The nested shapes are also exported/reachable on their own; strictness + // must not depend on arriving through ETLPipelineSchema. + expect(ETLSourceSchema.safeParse({ type: 'database', config: {}, nope: 1 }).success).toBe(false); + expect(ETLDestinationSchema.safeParse({ type: 'database', config: {}, nope: 1 }).success).toBe(false); + expect(ETLTransformationSchema.safeParse({ type: 'map', config: {}, nope: 1 }).success).toBe(false); + // …and the same three documents WITHOUT the key still parse. + expect(ETLSourceSchema.safeParse({ type: 'database', config: {} }).success).toBe(true); + expect(ETLDestinationSchema.safeParse({ type: 'database', config: {} }).success).toBe(true); + expect(ETLTransformationSchema.safeParse({ type: 'map', config: {} }).success).toBe(true); + }); +}); + +describe('[#4001 批 12] curated prescriptions — each anchored to a sibling contract', () => { + it('renames the connector layer’s `timestampField` to `cursorField`', () => { + // Anchor: integration/connector.zod.ts DataSyncConfig.timestampField. + expect(rejectionFor(['source', 'incremental'], 'timestampField')) + .toContain('`timestampField` → `cursorField`'); + }); + + it('resolves a borrowed `strategy` to a DIFFERENT key per surface', () => { + // One connector enum (`full | incremental | upsert | append_only`) splits + // across two keys here. A single global alias would confidently misdirect + // one of the two surfaces, so this pair is the load-bearing assertion. + expect(rejectionFor(['destination'], 'strategy')).toContain('`strategy` → `writeMode`'); + expect(rejectionFor([], 'strategy')).toContain('`strategy` → `syncMode`'); + }); + + it('renames `onError` to `onFailure` on the notification block', () => { + expect(rejectionFor(['notifications'], 'onError')).toContain('`onError` → `onFailure`'); + }); + + it('renames `maxRetries` to `maxAttempts`, and points `retryDelayMs` at `backoffMs`', () => { + expect(rejectionFor(['retry'], 'maxRetries')).toContain('`maxRetries` → `maxAttempts`'); + const retired = rejectionFor(['retry'], 'retryDelayMs'); + expect(retired).toContain('backoffMs'); + expect(retired).toContain('#4661'); + }); + + it('names the three converged-policy keys this block deliberately lacks (#4962)', () => { + for (const absent of ['backoffMultiplier', 'maxRetryDelayMs', 'jitter']) { + const message = rejectionFor(['retry'], absent); + expect(message, `${absent} must carry the absence prescription`).toContain('documented ABSENCE'); + expect(message).toContain('#4962'); + } + }); + + it('explains that pipeline direction is structural, not a key', () => { + const message = rejectionFor([], 'direction'); + expect(message).toContain('no `direction` key'); + expect(message).toContain('swap the two endpoints'); + // A guidance entry SUPPRESSES the rename suggestion — the author is told + // the mechanism, not sent to a key that does not mean the same thing. + expect(message).not.toContain('Did you mean'); + }); + + it('never suggests a key the schema will not accept', () => { + // The helper's `acceptsNothing` rule, asserted from this file's side: every + // key named in a "Did you mean X" must itself parse when written. + const message = rejectionFor([], 'sourse') ?? ''; + const suggested = [...message.matchAll(/→ `([^`]+)`/g)].map((m) => m[1]); + expect(suggested).toContain('source'); + for (const key of suggested) { + expect(Object.keys(VALID_PIPELINE), `${key} must be a real, writable key`).toContain(key); + } + }); +}); + +describe('[#4001 批 12] the three wire shapes stay tolerant — deliberate, and pinned', () => { + // If a later sweep closes these, THESE tests are what must be consciously + // deleted. That is the point: the exemption is a decision with a receipt, not + // an omission (see the comment on ETLPipelineRunSchema). + it('parses the base run result — positive control', () => { + expect(ETLPipelineRunSchema.safeParse(structuredClone(VALID_RUN)).success).toBe(true); + }); + + it.each([ + ['ETLPipelineRunSchema', [] as string[]], + ['ETLPipelineRunSchema.stats', ['stats']], + ['ETLPipelineRunSchema.error', ['error']], + ])('%s forwards an engine-added key instead of crashing', (_label, path) => { + const doc = structuredClone(VALID_RUN) as Record; + let cursor: Record = doc; + for (const step of path) cursor = cursor[step] as Record; + // The realistic case: a future engine reports one more counter. On a strict + // shape that is a parse CRASH for every existing reader — the #3712 shape. + cursor.recordsSkippedByANewerEngine = 7; + expect(ETLPipelineRunSchema.safeParse(doc).success).toBe(true); + }); + + it('still validates what it does declare — tolerance is not absence of a contract', () => { + // Anti-vacuity: the pins above must not be passing because the schema + // validates nothing at all. + expect(ETLPipelineRunSchema.safeParse({ ...VALID_RUN, status: 'completed' }).success).toBe(false); + expect(ETLPipelineRunSchema.safeParse({ ...VALID_RUN, startedAt: 'not-a-date' }).success).toBe(false); + expect(ETLPipelineRunSchema.safeParse({ id: 'r' }).success).toBe(false); + }); +}); + +describe('[#4001 批 12] strictness does not disturb the published contract', () => { + it('converts to JSON Schema through the lazy proxy without throwing (#3746 hazard)', async () => { + const { z } = await import('zod'); + for (const schema of [ + ETLSourceSchema, ETLDestinationSchema, ETLTransformationSchema, + ETLPipelineSchema, ETLPipelineRunSchema, + ]) { + // `io: 'input'` is asserted rather than the default because + // `ETLPipelineSchema` does not convert in OUTPUT mode — `schedule` is + // `CronExpressionInputSchema`, a transform, and "Transforms cannot be + // represented in JSON Schema". That is PRE-EXISTING and unrelated to + // strictness (verified: the same throw on the pre-批-12 file), and + // `build-schemas.ts` already handles it by falling back to input mode. + // Asserting the default here would have pinned someone else's known + // limitation as if this batch owned it. + expect(() => z.toJSONSchema(schema as never, { io: 'input' })).not.toThrow(); + } + }); + + /** + * The campaign's standing claim is that strictness does not move the + * published JSON Schema: `build-schemas.ts` converts with `io: 'output'`, and + * OUTPUT mode already emits `additionalProperties: false` for a `.strip()` + * object (pinned in `shared/strict-object.test.ts`). + * + * That claim holds per-direction, and this file is the case where the + * direction is not the usual one. `ETLPipelineSchema` cannot convert in + * output mode at all — `schedule` is `CronExpressionInputSchema`, a transform + * — so `build-schemas.ts` falls back to `io: 'input'`, and INPUT mode + * distinguishes the two postures: strip emits nothing, strict emits `false`. + * + * So for this one schema the batch DOES narrow the published contract, from + * "unspecified" to "closed". That is the intended direction (the publication + * now matches the parse instead of being quieter than it) but it is a real + * artifact change, not a no-op, and pretending otherwise is how a generated + * baseline moves without anyone reading it. Pinned here in both directions so + * the distinction survives the next person who quotes the flat claim. + */ + it('narrows the published schema only where input-mode fallback applies', async () => { + const { z } = await import('zod'); + const json = (s: unknown, io: 'input' | 'output') => + z.toJSONSchema(s as never, { io }) as Record; + + // The seven, via the pipeline: closed in the direction that gets published. + expect(json(ETLPipelineSchema, 'input').additionalProperties).toBe(false); + + // The three: still `z.object`, so they follow strip's per-direction shape — + // open in input mode, `false` in output mode. This is the receipt that the + // wire exemption is real at the schema level and not only in the parse. + expect(json(ETLPipelineRunSchema, 'input').additionalProperties).toBeUndefined(); + expect(json(ETLPipelineRunSchema, 'output').additionalProperties).toBe(false); + }); + + it('leaves both ETL factories producing documents that parse', () => { + // The factories construct pipelines programmatically — exactly the caller a + // newly-strict schema would break if the campaign had guessed a key wrong. + expect(ETLPipelineSchema.safeParse(ETL.databaseSync({ + name: 'users_sync', sourceTable: 'src', destTable: 'dst', schedule: '0 * * * *', + })).success).toBe(true); + expect(ETLPipelineSchema.safeParse(ETL.apiToDatabase({ + name: 'api_ingest', apiConnector: 'stripe', destTable: 'payments', + })).success).toBe(true); + }); +}); diff --git a/packages/spec/src/automation/etl.zod.ts b/packages/spec/src/automation/etl.zod.ts index db56d8e030..c98dc52dbe 100644 --- a/packages/spec/src/automation/etl.zod.ts +++ b/packages/spec/src/automation/etl.zod.ts @@ -2,6 +2,7 @@ import { z } from 'zod'; import { CronExpressionInputSchema } from '../shared/expression.zod'; +import { strictObject } from '../shared/strict-object'; /** * ETL (Extract, Transform, Load) Pipeline Protocol - LEVEL 2: Data Engineering @@ -99,10 +100,174 @@ export const ETLEndpointTypeSchema = lazySchema(() => z.enum([ export type ETLEndpointType = z.infer; +// ─── Unknown-key strictness (#4001 批 12, ADR-0078) ─────────────────── +// +// The seven AUTHORING shapes in this file are closed against undeclared keys; +// the three `ETLPipelineRun` shapes at the bottom stay tolerant. The split, and +// how it was verified rather than assumed, is recorded on `ETLPipelineRunSchema` +// — read it before closing anything else here. +// +// One thing to know about this file before reading the tables: `etl.zod.ts` has +// **no parse site anywhere** in objectstack / objectui / cloud. That does not +// make it unauthored — it makes the exported schema and the exported type the +// whole authoring door (`const p: ETLPipeline = { … }`, as +// `packages/spec/docs/SYNC_ARCHITECTURE.md` and this module's own `@example` +// write it), the same posture the ledger already carries for `webhook.zod.ts`. +// It does mean the curation below could not be measured from stored payloads +// the way 批 9's was, because there are none. So every alias and guidance entry +// here is instead anchored to a **sibling contract that exists in this repo** +// and spells the same intent differently — each one names its anchor. Nothing +// is written from imagination: the campaign's finding 7 is that a confidently +// wrong prescription costs more than no prescription at all. + +/** + * The shared second half of the endpoint/transformation histories: where a + * misplaced setting actually goes. + * + * `source`, `destination` and each `transformation` all pair a small closed key + * set with an open `config: z.record(…)` bag, and that pairing is what makes an + * unknown key on these three a **misplacement** far more often than a typo. + * `table`, `schema`, `endpoint`, `path`, `format`, `condition`, `groupBy` are + * all real, load-bearing settings — one nesting level down. `.strip` deleted + * them where they stood, so the pipeline parsed clean and then ran against a + * `config` missing exactly the setting the author had written. + * + * The pointer lives in `history` — appended to *every* unknown-key message on + * these surfaces — rather than in a per-key `guidance` table, because the + * misplaced key is drawn from the open bag's unbounded vocabulary. Enumerating + * it would be guesswork; naming the destination is not. + */ +const ETL_CONFIG_SLOT_POINTER = + 'If the key is a real setting (`table`, `schema`, `endpoint`, `path`, `format`, `condition`, `groupBy`, …) ' + + 'it belongs one level down, inside the open `config` record — that bag is deliberately unconstrained and ' + + 'is the only place this schema reads endpoint-specific settings from.'; + +const ETL_SOURCE_HISTORY = + 'Until #4001 an undeclared key on an ETL source was dropped silently — the pipeline parsed clean and ' + + 'extracted with the key ignored. ' + ETL_CONFIG_SLOT_POINTER; + +const ETL_INCREMENTAL_HISTORY = + 'Until #4001 an undeclared key here was dropped silently, and an incremental source whose cursor never ' + + 'took effect re-extracts the whole table (or nothing) on every run while still reporting success.'; + +/** + * Anchor: `DataSyncConfig` in `integration/connector.zod.ts` — the LIVE sibling + * (it is on the `ConnectorSchema.syncConfig` parse path) — calls this same thing + * `timestampField`, "Field to track last modification time". Identical intent, + * different word, and far outside the edit-distance window, which is exactly + * the category `aliases` exists for. + */ +const ETL_INCREMENTAL_ALIASES: Readonly> = { + timestampField: 'cursorField', +}; + +const ETL_DESTINATION_HISTORY = + 'Until #4001 an undeclared key on an ETL destination was dropped silently — the pipeline parsed clean and ' + + 'loaded with the key ignored. ' + ETL_CONFIG_SLOT_POINTER; + +/** + * Anchor: connector `syncConfig.strategy` (`SyncStrategySchema`, + * `integration/connector.zod.ts`) is ONE enum — `full | incremental | upsert | + * append_only` — whose four values split across TWO keys on this file: the + * write half (`upsert` / `append_only`) is the destination's `writeMode`, the + * extraction half (`full` / `incremental`) is the pipeline's `syncMode`. + * + * So the same borrowed word resolves to a different canonical key depending on + * which surface it was written on, and each surface names only its own half. + * Getting that right is the whole value of a hand-written alias here — a + * single global "strategy → syncMode" would send a destination author to the + * wrong key with full confidence. + */ +const ETL_DESTINATION_ALIASES: Readonly> = { + strategy: 'writeMode', +}; + +const ETL_TRANSFORMATION_HISTORY = + 'Until #4001 an undeclared key on an ETL transformation was dropped silently — the step ran with the key ' + + 'ignored and the pipeline reported success. ' + ETL_CONFIG_SLOT_POINTER; + +const ETL_PIPELINE_HISTORY = + 'Until #4001 an undeclared key on an ETL pipeline was dropped silently — the pipeline parsed clean and ' + + 'ran, minus whatever the key was meant to configure.'; + +/** See {@link ETL_DESTINATION_ALIASES} — this is that enum's extraction half. */ +const ETL_PIPELINE_ALIASES: Readonly> = { + strategy: 'syncMode', +}; + +/** + * Anchor: `DataSyncConfig.direction` (`import | export | bidirectional`) is a + * declared key on the connector layer and a **documented absence** here — which + * makes it the likeliest wrong key for anyone arriving from that layer, exactly + * the shape 批 9 recorded for `outputVariable` on `update_record`. + */ +const ETL_PIPELINE_GUIDANCE: Readonly> = { + direction: + 'An ETL pipeline has no `direction` key, by design: direction is stated STRUCTURALLY, by which endpoint ' + + 'is `source` and which is `destination`. `direction` (import/export/bidirectional) is the CONNECTOR ' + + "layer's spelling (`ConnectorSchema.syncConfig`); to reverse an ETL pipeline, swap the two endpoints.", +}; + +const ETL_RETRY_HISTORY = + 'Until #4001 an undeclared key here was dropped silently and the block fell back to its defaults ' + + '(3 attempts, 60s) while reporting the authored policy as accepted.'; + +/** + * Anchor: `RetryPolicySchema` (`shared/retry-policy.zod.ts`) — the retry policy + * #4661 converged onto ONE declaration for `job.retryPolicy` and a `try_catch` + * node's `retry` region. This block is a **third** encoding of the same concept + * that the convergence did not reach, because it is an anonymous inline object + * with no exported name and so never appeared in the #4411 / #4535 dual-source + * scan that drove that work. + * + * Closing this shape does not fix the divergence — it makes it audible. The + * five entries below are the whole diff between the two vocabularies, stated + * where an author hits it. Whether to converge (and which default `maxAttempts` + * should then take: #4661 argues 0, this block ships 3) is #4962 — a contract + * decision, deliberately not made inside a strictness batch. + */ +const ETL_RETRY_ALIASES: Readonly> = { + maxRetries: 'maxAttempts', +}; + +/** The three keys the converged policy declares and this block deliberately does not. */ +const etlRetryAbsence = (key: string): string => + `\`${key}\` is declared on the converged \`RetryPolicySchema\` (\`shared/retry-policy.zod.ts\`, #4661) but ` + + `NOT on an ETL pipeline's \`retry\`, which declares only \`maxAttempts\` + \`backoffMs\` — a flat, uncapped, ` + + `unjittered backoff. This is a documented ABSENCE, not a typo: nothing here would read the key. Converging ` + + `the two vocabularies is tracked as #4962.`; + +const ETL_RETRY_GUIDANCE: Readonly> = { + retryDelayMs: + '`retryDelayMs` was the pre-17 automation-side spelling of the base delay and was removed in ' + + '@objectstack/spec 17.0.0 (#4661); it is tombstoned on `RetryPolicySchema`. This block already spells it ' + + '`backoffMs` — rename the key, the value (milliseconds before the first retry) is unchanged.', + backoffMultiplier: etlRetryAbsence('backoffMultiplier'), + maxRetryDelayMs: etlRetryAbsence('maxRetryDelayMs'), + jitter: etlRetryAbsence('jitter'), +}; + +const ETL_NOTIFICATIONS_HISTORY = + 'Until #4001 an undeclared key here was dropped silently — nobody was notified and the run still reported ' + + 'success, which is the one outcome this block exists to prevent.'; + +/** + * Anchor: three in-repo surfaces spell the failure hook `onError` + * (`ui/widget.zod.ts`, `data/hook.zod.ts`'s declared key list, + * `kernel/plugin-loading.zod.ts`). This block spells it `onFailure`, and the + * two are six edits apart — unreachable by the distance fallback. + */ +const ETL_NOTIFICATIONS_ALIASES: Readonly> = { + onError: 'onFailure', +}; + /** * ETL Source Configuration */ -export const ETLSourceSchema = lazySchema(() => z.object({ +export const ETLSourceSchema = lazySchema(() => strictObject({ + surface: 'this ETL source', + history: ETL_SOURCE_HISTORY, +}, { /** * Source type */ @@ -130,7 +295,11 @@ export const ETLSourceSchema = lazySchema(() => z.object({ * Incremental sync configuration * Allows extracting only changed data */ - incremental: z.object({ + incremental: strictObject({ + surface: 'this ETL incremental extraction config', + history: ETL_INCREMENTAL_HISTORY, + aliases: ETL_INCREMENTAL_ALIASES, + }, { enabled: z.boolean().default(false), cursorField: z.string().describe('Field to track progress (e.g., updated_at)'), cursorValue: z.unknown().optional().describe('Last processed value'), @@ -142,7 +311,11 @@ export type ETLSource = z.infer; /** * ETL Destination Configuration */ -export const ETLDestinationSchema = lazySchema(() => z.object({ +export const ETLDestinationSchema = lazySchema(() => strictObject({ + surface: 'this ETL destination', + history: ETL_DESTINATION_HISTORY, + aliases: ETL_DESTINATION_ALIASES, +}, { /** * Destination type */ @@ -197,7 +370,17 @@ export type ETLTransformationType = z.infer; /** * ETL Transformation Configuration */ -export const ETLTransformationSchema = lazySchema(() => z.object({ +export const ETLTransformationSchema = lazySchema(() => strictObject({ + surface: 'this ETL transformation', + history: ETL_TRANSFORMATION_HISTORY, + // No curated table. Every transformation-specific setting this file or + // SYNC_ARCHITECTURE.md ever writes (`condition`, `groupBy`, `joinKey`, + // `joinType`, `metrics`, `language`, `code`) lives inside the open `config` + // bag, so the pointer in `history` already answers them as a class; and this + // surface has no sibling contract spelling one of its four declared keys + // differently, so there is nothing an alias could honestly claim. Same + // reasoning as `HttpConfigSchema` in `io-node-config.zod.ts` (#4001 批 9). +}, { /** * Transformation name */ @@ -241,7 +424,12 @@ export type ETLSyncMode = z.infer; * * Complete definition of a data pipeline from source to destination with transformations. */ -export const ETLPipelineSchema = lazySchema(() => z.object({ +export const ETLPipelineSchema = lazySchema(() => strictObject({ + surface: 'this ETL pipeline', + history: ETL_PIPELINE_HISTORY, + aliases: ETL_PIPELINE_ALIASES, + guidance: ETL_PIPELINE_GUIDANCE, +}, { /** * Pipeline identifier (snake_case) */ @@ -299,7 +487,12 @@ export const ETLPipelineSchema = lazySchema(() => z.object({ /** * Retry configuration for failed runs */ - retry: z.object({ + retry: strictObject({ + surface: "this ETL pipeline's retry configuration", + history: ETL_RETRY_HISTORY, + aliases: ETL_RETRY_ALIASES, + guidance: ETL_RETRY_GUIDANCE, + }, { maxAttempts: z.number().int().min(0).default(3).describe('Max retry attempts'), backoffMs: z.number().int().min(0).default(60000).describe('Backoff in milliseconds'), }).optional().describe('Retry configuration'), @@ -307,7 +500,11 @@ export const ETLPipelineSchema = lazySchema(() => z.object({ /** * Notification configuration */ - notifications: z.object({ + notifications: strictObject({ + surface: "this ETL pipeline's notification settings", + history: ETL_NOTIFICATIONS_HISTORY, + aliases: ETL_NOTIFICATIONS_ALIASES, + }, { onSuccess: z.array(z.string()).optional().describe('Email addresses for success notifications'), onFailure: z.array(z.string()).optional().describe('Email addresses for failure notifications'), }).optional().describe('Notification settings'), @@ -341,8 +538,43 @@ export type ETLRunStatus = z.infer; /** * ETL Pipeline Run Result - * - * Result of a pipeline execution + * + * Result of a pipeline execution. + * + * ## Deliberately NOT strict — the wire half of this file (#4001 批 12) + * + * Everything above this line closed against unknown keys. These three shapes — + * `ETLPipelineRunSchema` and its `stats` / `error` blocks — stay tolerant, and + * this comment is the exemption record so the next sweep reads a decision + * rather than an omission. Same disposition, same reason, as + * `FlowVersionHistorySchema` in `flow.zod.ts` and the whole of + * `execution.zod.ts` ("run-state envelopes — never strict"). + * + * **Why.** Every key here is a fact the engine PRODUCES about a run that + * already happened: an id it minted, a status it reached, timestamps it + * observed, counters it accumulated, the error it caught. Nobody authors a run + * result — writing one by hand is not a use case, it is a lie about history. + * Strictness on a shape like this buys nothing (there is no author to protect + * from a silent strip) and costs the thing the campaign is most careful about: + * an engine that later reports one more counter would turn every existing + * reader's `.parse()` into a crash, which is how a tolerant wire shape becomes + * a breaking change by accident (#3712 did exactly this to `provenance` on + * `HookContextSchema`, and the ledger's `hook.zod.ts` row records the same + * split for the same reason). + * + * **How this was verified, and the limit of that verification.** The honest + * measurement is stated rather than dressed up: `etl.zod.ts` has NO parse site + * in objectstack, objectui or cloud, so neither half of this file could be + * classified by pointing at a live call. The seven above are authorable because + * the exported schema and type ARE the authoring door — `SYNC_ARCHITECTURE.md` + * and this module's `@example` both write `const p: ETLPipeline = { … }` by + * hand. These three are wire because their key set is engine-produced fact, and + * because no ETL engine exists yet the argument rests on the shape's semantics + * plus the campaign's settled precedent for exactly this pair — not on an emit + * site anyone can point at today. **The day an ETL engine lands, this comment + * is the thing to re-read**: if a run result turns out to be something an + * operator authors (a replay stub, a backfill marker), the verdict changes and + * the ledger row changes with it. */ export const ETLPipelineRunSchema = lazySchema(() => z.object({ /**