From 3094a1713e9976fb5f163d06eaf901a482ba036e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 17:45:07 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat(spec)!:=20reject=20unknown=20keys=20on?= =?UTF-8?q?=20the=20ETL=20authoring=20contracts=20(#4001=20=E6=89=B9=2012)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven strip sites in `automation/etl.zod.ts` close — `ETLSource` (+ its `.incremental` block), `ETLDestination`, `ETLTransformation`, and `ETLPipeline` (+ its `.retry` and `.notifications` blocks). `automation/` remaining-strip: 53 → 46 (authorable 27 → 20). The file's other three sites — `ETLPipelineRun` + `.stats` + `.error` — are deliberately left OPEN. Every key on them is a fact the engine produces about a run that already happened; nobody authors a run result, so strictness buys no author protection there and would turn a future counter into a parse crash for every existing reader (the #3712 shape). Same disposition, same reason, as `FlowVersionHistorySchema` and all of `execution.zod.ts`. The exemption is recorded on the schema itself and pinned in `etl.test.ts`, not only in the ledger — a note only the ledger carries is a note the next sweep misses. Verified before tightening, and the verification has a stated limit: this file has NO parse site in objectstack / objectui / cloud, so neither half could be settled by pointing at a live call. The seven are authorable because the exported schema and type ARE the authoring door (SYNC_ARCHITECTURE.md and the module's own `@example` both hand-write `const p: ETLPipeline = { … }`) — the same posture the ledger already records for `webhook.zod.ts`. The three are wire on the shapes' semantics plus settled precedent, not on an emit site. Curation is anchored, not invented. With no stored payloads to scan, every alias and guidance entry names a sibling contract in this repo that spells the same intent differently: `timestampField` → `cursorField` (connector `DataSyncConfig`), `onError` → `onFailure` (three in-repo surfaces), `maxRetries` → `maxAttempts` and the retired `retryDelayMs` → `backoffMs` (the converged `shared/RetryPolicySchema`, #4661). The connector's one `strategy` enum splits across two keys here, so it resolves to `writeMode` on a destination and `syncMode` on a pipeline — a single global alias would misdirect one of them with full confidence. `direction` gets a documented absence rather than a rename: an ETL pipeline states direction structurally. The dominant failure on this file is misplacement, not typing — `table`, `endpoint`, `path`, `condition` are real settings one level down, inside the open `config` bag. That pointer lives in `history` (appended to every message) rather than a per-key table, because the open bag's vocabulary is unbounded and enumerating it would be guesswork. One published-artifact consequence, stated rather than glossed: the campaign's "strictness does not move the JSON Schema" claim is per-direction, and `ETLPipelineSchema` is the case where the usual direction does not apply — it cannot convert in output mode (`schedule` is a transform), so build-schemas falls back to input mode, where strip emits nothing and strict emits `additionalProperties: false`. The publication now matches the parse; it is a real narrowing, not a no-op, and it is pinned in both directions. Two out-of-scope findings filed rather than fixed: #4962 (`retry` is a third retry-policy vocabulary #4661's convergence never reached) and #4963 (all nine type aliases export the parsed shape under the bare name, which is why the SYNC_ARCHITECTURE.md pipeline examples do not compile). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --- .../2026-07-unknown-key-strictness-ledger.md | 26 +- packages/spec/src/automation/etl.test.ts | 274 ++++++++++++++++++ packages/spec/src/automation/etl.zod.ts | 250 +++++++++++++++- 3 files changed, 535 insertions(+), 15 deletions(-) diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index d880a839ef..a784275148 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -528,7 +528,7 @@ not verdicts). | File | Sites | Class | Note | |---|---|---|---| | `flow.zod.ts` | 11 | authorable | **strict as of #4001** (4 schemas; `FlowVersionHistorySchema` is runtime — stays tolerant) | -| `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 (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 | | `control-flow.zod.ts` | 5 | authorable (p) | validated structurally by `validateControlFlow`. **−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 | @@ -605,12 +605,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/` — 53 strip of 75 +#### `automation/` — 46 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 is the first in `automation/` to shrink without disappearing**, which is worth naming: the reverse pin can only tell you a file reached zero, so a row that stops at its wire floor looks identical to a row nobody finished. The Class column is the only thing separating them — read it before treating this as unfinished work | | `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) | | `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 | @@ -626,10 +626,24 @@ wave — `builtin-node-config.zod.ts` (8), `schemaless-node-config.zod.ts` (4) a all three before the rows were removed, which is the only evidence that a deletion here is bookkeeping rather than a guess. -**Authorable strip in `automation/`: 27 of 53** (was 41 of 67). What remains of -the ruling's "known main body" is `etl` 7, `flow` 6, `state-machine` 6, +**批 12** closed `etl.zod.ts`'s authorable half (7) and is the first row here to +**shrink without disappearing** — it stops at a wire floor of 3 rather than +reaching zero. That exercises the half of the bookkeeping 批 9 could not: the +reverse pin proves a row's work is *done*, and it is silent about a row whose +work is *deliberately partial*. Only the `Class` column distinguishes "finished, +3 wire sites remain by decision" from "nobody got to it", so from 批 12 on the +Class column is load-bearing rather than descriptive — and the decision is also +written on the schema in `etl.zod.ts` and pinned in `etl.test.ts`, because a +reader who arrives via the code will never open this file. + +**Authorable strip in `automation/`: 20 of 46** (was 27 of 53, and 41 of 67). +What remains of the ruling's "known main body" is `flow` 6, `state-machine` 6, `control-flow` 5, and one each from `flow-function` / `time-relative-trigger` / -`webhook`. +`webhook`. ⚠️ 批 10 and 批 11 are in flight against exactly those rows as this is +written, so expect this paragraph and the section header to conflict three ways; +the resolution is always **keep every side's row edits and recompute the header +from the rows**, then let `check:strictness-ledger` arbitrate — it reads the +code, not the prose. #### `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({ /** From ee1678383611d4a06da730b81f292e06298ddee3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 18:10:25 +0000 Subject: [PATCH 2/2] =?UTF-8?q?docs(spec):=20changeset=20for=20the=20ETL?= =?UTF-8?q?=20strictness=20batch=20(#4001=20=E6=89=B9=2012)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --- .../unknown-key-strictness-automation-etl.md | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 .changeset/unknown-key-strictness-automation-etl.md 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.