diff --git a/.changeset/import-run-automations-declared-default.md b/.changeset/import-run-automations-declared-default.md new file mode 100644 index 0000000000..8d844b5d61 --- /dev/null +++ b/.changeset/import-run-automations-declared-default.md @@ -0,0 +1,60 @@ +--- +"@objectstack/spec": major +--- + +fix(spec)!: `ImportRequest.runAutomations` declares the default the import route actually applies (#6704, ADR-0049) + +`POST /api/v1/data/:object/import` — and its async twin `POST /api/v1/data/:object/import/jobs` — +has fired triggers and hooks for an omitted `runAutomations` since #2922. The server +decides with `body?.runAutomations !== false`: automations always ran on import +historically, so opting **out** was made the explicit act, matching platform +convention (Salesforce fires triggers on import by default). + +The schema declared the opposite, and said so twice. `.default(false)` shipped in +`@objectstack/spec`'s JSON Schema, and the `describe` prose — "off by default for +bulk" — rendered into the published reference tables for **both** defs +(`ImportRequest` and `CreateImportJobRequest`). Both are now corrected to the +runtime: `.default(true)`, with prose that states automations run by default and +that opt-out must be explicit. + +**Runtime behaviour is deliberately UNCHANGED.** `packages/rest/src/import-prepare.ts` +is untouched by this change. Nothing starts being refused, and no request that +worked before behaves differently on the wire. + +### Why a wrong declaration was reachable at all + +Nothing in the repo reconciled the two halves, which is why no gate could see the +divergence: no request path parses an import body through this schema. The route +reads the raw body, and the single reference to `CreateImportJobRequestSchema` is +the declarative `ImportJobApiContracts` catalog entry — a declaration, not a parse. +Each half was internally consistent; only their disagreement was wrong. + +### Migration: FROM → TO + +| FROM | TO | +| :--- | :--- | +| omitting `runAutomations` and expecting no triggers, because the schema said so | send `runAutomations: false` — the only spelling the server has ever read | +| omitting it and expecting triggers | change nothing; that is what you already got, and now what is declared | +| reading `ImportRequestParsed.runAutomations` after parsing a body without the key | it now yields `true` instead of `false` — the value the server would have applied anyway | + +**Who is actually affected:** a client or SDK that validates its request through the +published schema and sends the **parsed** object. It materialised +`runAutomations: false` from the declared default and sent it explicitly, and the +server honoured that — so identical request bodies produced opposite behaviour +depending on whether the caller validated before sending, with the validating +caller silently losing its triggers. Those bulk loads ran with automations off and +will now run with them on, which is what an unvalidated caller always got. A caller +that never parsed its own request body is unaffected in every direction. + +`dryRun` is untouched and still runs **no** automations whatever this flag says +(#6037). + +Maintainer ruling 2026-08-09 (#6704), disposition A — the spec follows the runtime: + +> **Maintainer ruling (2026-08-09): disposition A — the spec follows the runtime.** `ImportRequest.runAutomations` becomes `.default(true)` with corrected describe prose (state that automations run by default and opt-out must be explicit, per the #2922 rationale); the generated reference tables follow. Runtime behaviour unchanged. [...] Changeset notes the declared-default flip of a published schema (a correction toward the actual shipped behaviour, not a behaviour change). + +The declared move itself is recorded per key in `DEFAULT_CHANGES_BY_MAJOR[17]`, whose +`from`/`to` fingerprints are re-derived on every build, so the declaration cannot +outlive the fact it describes. + + diff --git a/content/docs/references/api/export.mdx b/content/docs/references/api/export.mdx index 8b9fba1bd4..3a54e53f74 100644 --- a/content/docs/references/api/export.mdx +++ b/content/docs/references/api/export.mdx @@ -79,7 +79,7 @@ const result = CreateExportJobRequestSchema.parse(data); | **dryRun** | `boolean` | ✅ | Validate + coerce every row without persisting. The verdict is the engine's own write-path validation, with one boundary an author should know: a preview runs NO automations. Hooks never fire in a dry run (#6037) — a preview that executed user-authored side effects (mail, outbound calls, writes to other objects) would be the retired `validateOnly` defect in a new spelling. So a dry run with `runAutomations: true` can report `required` for a field a `beforeInsert` hook would populate during the real import; for hook-derived fields the real write is authoritative. | | **writeMode** | `Enum<'insert' \| 'update' \| 'upsert'>` | ✅ | insert / update / upsert semantics | | **matchFields** | `string[]` | optional | Fields that identify an existing record (required for update/upsert) | -| **runAutomations** | `boolean` | ✅ | Fire triggers/hooks for each imported row (off by default for bulk) | +| **runAutomations** | `boolean` | ✅ | Fire triggers/hooks for each imported row. ON by default, and opting out must be explicit: automations always ran on import historically (the engine ignored this flag until #2922), so a caller that wants a silent bulk load sends `runAutomations: false` — omitting the key runs them. This matches platform convention (Salesforce fires triggers on import by default). One boundary: a `dryRun` preview runs NO automations whatever this flag says (#6037). | | **treatAsHistorical** | `boolean` | ✅ | Import as established historical facts. Two effects, both off by default so a normal import is unchanged: (1) skip the state_machine rule so mid-lifecycle rows (e.g. already-closed tickets, closed_won deals) are not rejected by initialStates (#3479); and (2) preserve the original audit timeline — keep the supplied created_at / updated_at / updated_by and author-declared business readonly fields (e.g. closed_at, resolved_by) instead of stamping-now / stripping them (#3493). Undoing a historical import mirrors (2): the captured pre-import values are restored verbatim rather than re-stamped (#3556). | | **trimWhitespace** | `boolean` | ✅ | Trim leading/trailing whitespace from string cells | | **nullValues** | `string[]` | optional | Strings treated as null/blank (besides empty string) | @@ -368,7 +368,7 @@ Type: `{ sourceField: string; targetField: string; targetLabel?: string; transfo | **dryRun** | `boolean` | ✅ | Validate + coerce every row without persisting. The verdict is the engine's own write-path validation, with one boundary an author should know: a preview runs NO automations. Hooks never fire in a dry run (#6037) — a preview that executed user-authored side effects (mail, outbound calls, writes to other objects) would be the retired `validateOnly` defect in a new spelling. So a dry run with `runAutomations: true` can report `required` for a field a `beforeInsert` hook would populate during the real import; for hook-derived fields the real write is authoritative. | | **writeMode** | `Enum<'insert' \| 'update' \| 'upsert'>` | ✅ | insert / update / upsert semantics | | **matchFields** | `string[]` | optional | Fields that identify an existing record (required for update/upsert) | -| **runAutomations** | `boolean` | ✅ | Fire triggers/hooks for each imported row (off by default for bulk) | +| **runAutomations** | `boolean` | ✅ | Fire triggers/hooks for each imported row. ON by default, and opting out must be explicit: automations always ran on import historically (the engine ignored this flag until #2922), so a caller that wants a silent bulk load sends `runAutomations: false` — omitting the key runs them. This matches platform convention (Salesforce fires triggers on import by default). One boundary: a `dryRun` preview runs NO automations whatever this flag says (#6037). | | **treatAsHistorical** | `boolean` | ✅ | Import as established historical facts. Two effects, both off by default so a normal import is unchanged: (1) skip the state_machine rule so mid-lifecycle rows (e.g. already-closed tickets, closed_won deals) are not rejected by initialStates (#3479); and (2) preserve the original audit timeline — keep the supplied created_at / updated_at / updated_by and author-declared business readonly fields (e.g. closed_at, resolved_by) instead of stamping-now / stripping them (#3493). Undoing a historical import mirrors (2): the captured pre-import values are restored verbatim rather than re-stamped (#3556). | | **trimWhitespace** | `boolean` | ✅ | Trim leading/trailing whitespace from string cells | | **nullValues** | `string[]` | optional | Strings treated as null/blank (besides empty string) | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 8595b3734f..552874ccc2 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -408,6 +408,9 @@ The same descriptor loses a key in this step, and the pairing is the point (#674 - **`notification-list-cursor-retired`** — `api.listNotifications cursor — the key on BOTH halves of GET /api/v1/notifications (ListNotificationsRequestSchema and ListNotificationsResponseSchema) and the cursor argument of the client SDK call client.notifications.list(). The same entry covers the limit default: the request schema no longer declares default(20)` → a larger `limit` — the route answers the newest N notifications and has no page 2. There is no replacement for `cursor`, deliberately: nothing ever minted one, so no caller holds a value to carry over. Callers that looped on it were re-reading the first window and should read one window sized to what they display (the Console bell polls exactly this way). For the removed `limit` default, send the number you want explicitly if you were relying on 20 — omitting it takes the server window, which is 50 on the platform inbox and clamped into 1..200, and has been since before the declaration existed - Why not automatic: One capability, both halves, never half-deleted (maintainer ruling 2026-08-07, Option A, ruled jointly with #6363). `cursor` was declared on the request and on the response and honoured on neither: the dispatcher domain reads `read` / `type` / `limit` and nothing else, and no emit site has ever written the response key. It was worse than inert because it had a shipped PRODUCER — the SDK appended it to the query string — so a caller paginating by the published contract looped on page 1 forever, with no error and no 400. Measured over a real boot with 60 unread before the removal: page2 === page1, both parsing green against the response schema, which is why no conformance gate could see it. This is `data.query.cursor` (#4286, `query-cursor-retired`) one layer up, with the same verdict for the same reason, down to deleting the SDK producer alongside the key. A first-class inbox cursor, if one is ever designed, will be a response-minted opaque token — a different API — so keeping this one preserved a wrong design rather than a roadmap. The `limit` default goes with it because the FICTION WAS THE MECHANISM, not the number: no request path parses a query string through this schema (#3899 wired the catalog's requestSchema to the real entry for BODIES only), so `.default(20)` never stamped anything onto anything, and the server has always applied its own 50. Re-spelling 20 as 50 — the other arm the ruling allowed — would have kept a declaration that does not execute and merely made it coincide with the implementation until someone moved the clamp; `.optional()` plus prose is true about both the schema and the server. No constraint (`.int()` / `.max(200)`) is declared either, because the service CLAMPS an out-of-range limit rather than refusing it, and declaring a rejection the wire does not perform is the same defect mirrored. Route 2, and the split is worth stating exactly because the two halves of the bookkeeping go different ways. There IS a tombstone: both schemas are non-strict, so a bare deletion would have made Zod SILENTLY STRIP whatever a caller kept sending — a clean parse and a parameter that never takes effect, which is this issue's own defect re-created one layer down (#3733, ADR-0104). So `cursor` is `retiredKey()` on both halves, typed `never` for tsc and raising the prescription at any parse, and both keys are registered in RETIRED_KEYS_BY_MAJOR[17]. There is NO D2 conversion: a conversion rewrites an authored source or a stored `sys_metadata` row, and these two shapes are HTTP-only — nobody authors a `ListNotificationsRequest` and nothing persists one. Request AND response shapes: two semantic TODOs for API callers, no stack conversion — the same disposition `BatchOptions.validateOnly` (#4052) and the `AnalyticsQueryRequest` envelope keys already take in this major. The `limit` default is declared separately and mechanically, in DEFAULT_CHANGES_BY_MAJOR[17] (#4666), whose `from`/`to` fingerprints are re-derived on every build. ADR-0049 / ADR-0078, #6361. - Done when: No caller sends `cursor` to `GET /api/v1/notifications` and no SDK call site passes it: `client.notifications.list({ cursor })` is a `tsc` error (TS2353, excess property), which is the enforced channel — the removal is loud at compile time for every TypeScript consumer. Reading `response.cursor` no longer type-checks either, and always answered `undefined` before. ⚠️ Behaviour on the wire is deliberately UNCHANGED and must be verified as such: a request still carrying `?cursor=…` is IGNORED, not refused — the domain reads three named query keys and no route validates this query against a schema, so an unknown key has never produced a 400 and does not start doing so here. The declaration stopped promising what the wire never did; the wire did not change. `unreadCount` is untouched (#6363) and still reports the total across the whole matching inbox rather than the window. A caller that omitted `limit` receives the same 50 rows it always received. +- **`import-run-automations-declared-default-corrected`** — `api.ImportRequest runAutomations — the declared default of the key on BOTH import bodies, POST /api/v1/data/:object/import (ImportRequest) and its async twin POST /api/v1/data/:object/import/jobs (CreateImportJobRequest, which IS the same schema object). It was declared default(false) and described as "off by default for bulk"; it is now default(true), which is what the server has always done` → an explicit runAutomations: false on any import request that is meant to load rows without firing triggers/hooks. That spelling is unchanged and has always been the only one the server read — what changes is that omitting the key now DECLARES what it already DID. Callers who want automations on need write nothing + - Why not automatic: A DECLARATION corrected to match a runtime that did not move — the inverse of a behaviour flip, and registered here for the reason protocol 12's `rest-requireauth-default-flip` and this major's `action-descriptor-resume-authority-default-flip` are: whether a given import was meant to fire triggers is a judgment no transform can make, so the prescription is a TODO rather than a rewrite. The server decides in import-prepare.ts with `body?.runAutomations !== false`, i.e. an omitted flag runs automations, and has since #2922 — automations always ran on import historically (the engine ignored the flag entirely before then), so opt-out was made the explicit act, matching platform convention. The schema said the opposite in both machine-readable and human-readable form, and both SHIPPED: `.default(false)` in `@objectstack/spec`'s JSON Schema, and the describe prose in the published reference tables for both defs. ⚠️ Nothing in this repo reconciled the two and NO deployed caller changes behaviour: no request path parses an import body through this schema — the route reads the raw body, and the sole reference to `CreateImportJobRequestSchema` is the declarative `ImportJobApiContracts` catalog entry, a declaration and not a parse. That is exactly why this needed a ruling rather than a docs edit: the divergence was unobservable in-tree and observable only to a consumer OUTSIDE it. A client or SDK that validated its request through the published schema materialised `runAutomations: false` from the declared default and sent it explicitly, and the server honoured it — so the same request body produced opposite behaviour depending on whether the caller validated before sending, with the validating caller silently losing its triggers. Nothing rejected it, nothing warned, and the reference page told an author the wrong thing in the other direction. There is deliberately NO schema tombstone and no D2 conversion: no key is removed, and an HTTP request body is neither authored nor persisted — the same disposition `notification-list-cursor-retired` (#6361) takes for the sibling default on this major, and `batch-options-validate-only-retired` before it. The declared move itself is recorded mechanically, per key, in DEFAULT_CHANGES_BY_MAJOR[17] (#4666), whose `from`/`to` fingerprints are re-derived on every build. Maintainer ruling 2026-08-09 (#6704, disposition A: the spec follows the runtime). ADR-0049 / ADR-0078. + - Done when: Every import request of yours that must NOT fire triggers sends `runAutomations: false` explicitly, rather than omitting the key and trusting the old declared default. The check is worth doing precisely where it looks unnecessary: if you build the body by parsing it through `ImportRequestSchema` (or the published JSON Schema) and then send the PARSED object, your bulk loads were running with automations OFF and will now run with them ON — that is the only class whose behaviour changes, and it changes toward what an unvalidated caller always got. ⚠️ Behaviour on the wire is deliberately UNCHANGED and should be verified as such: a body that omits `runAutomations` fired triggers before this change and fires them after, and `runAutomations: false` turns them off before and after. Nothing starts being refused — the route never validated this body against the schema and does not begin to. `dryRun` is unaffected and still runs NO automations whatever the flag says (#6037). --- diff --git a/packages/rest/src/import-run-automations-agreement.test.ts b/packages/rest/src/import-run-automations-agreement.test.ts new file mode 100644 index 0000000000..dab3ed1f30 --- /dev/null +++ b/packages/rest/src/import-run-automations-agreement.test.ts @@ -0,0 +1,119 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The declared default of `runAutomations` AGREES with the server's decision + * (#6704). + * + * ## Why the agreement, and not either half + * + * `ImportRequestSchema` declares what a caller who validates a request body + * materialises. `prepareImportRequest` decides what the server does with the + * body it is handed. Nothing in this repo connects the two — the import route + * reads the RAW body and never parses it through the schema, and the only + * reference to `CreateImportJobRequestSchema` is the declarative + * `ImportJobApiContracts` catalog entry, a declaration and not a parse. That + * missing connection is the whole defect: for two years the schema said + * `.default(false)` while the server ran `body?.runAutomations !== false`, and + * no gate could see it because each half was internally consistent. + * + * So neither half pins the fact. Asserting `parse({}).runAutomations === true` + * pins the schema; asserting `prepare({}).runAutomations === true` pins the + * server; only asserting they are EQUAL, over an input set that includes the + * omitted key, pins that the divergence is closed. This file is the only place + * in the repo that can make that assertion — `@objectstack/rest` depends on + * `@objectstack/spec`, so both halves are reachable from here and from nowhere + * upstream of it. + * + * ## Behaviour is deliberately UNCHANGED + * + * #6704 moved the DECLARATION to the runtime, never the runtime to the + * declaration: `packages/rest/src/import-prepare.ts` is untouched. The + * `serverDecision` column below therefore reads identically before and after the + * change, and the `declared` column is what moved. If a future edit "fixes" the + * disagreement from the other side — making an omitted flag skip automations — + * these cases stay green while `import-prepare.test.ts`'s own `#2922` block goes + * red, which is the correct division of labour between the two files. + */ + +import { describe, it, expect } from 'vitest'; +import { ImportRequestSchema, CreateImportJobRequestSchema } from '@objectstack/spec/api'; +import { prepareImportRequest } from './import-prepare'; + +const SCHEMA = { + name: 'task', + fields: { + title: { name: 'title', type: 'text', label: 'Title' }, + }, +}; + +const p = { getMetaItem: async () => ({ type: 'object', name: 'task', item: SCHEMA }) }; + +/** The rows are irrelevant to the flag; one valid row keeps `prepare` on its ok path. */ +const ROWS = [{ title: 'a' }]; + +/** What the server decides for a body, reading it exactly as the route does. */ +const serverDecision = async (body: Record): Promise => { + const prep = await prepareImportRequest( + { format: 'json', rows: ROWS, ...body }, + { p, objectName: 'task', maxRows: 10 }, + ); + expect(prep.ok).toBe(true); + if (!prep.ok) throw new Error('prepareImportRequest refused a valid body'); + return prep.prepared.runAutomations; +}; + +/** What a caller who validates the body first materialises and sends. */ +const declared = (body: Record): boolean => + ImportRequestSchema.parse({ format: 'json', rows: ROWS, ...body }).runAutomations; + +describe('runAutomations — declared default agrees with the server (#6704)', () => { + const cases: Array<{ label: string; body: Record; expected: boolean }> = [ + // THE case. Before #6704 this row was the divergence: declared `false`, + // server `true`. Everything else in this file already agreed. + { label: 'omitted', body: {}, expected: true }, + { label: 'explicit true', body: { runAutomations: true }, expected: true }, + { label: 'explicit false', body: { runAutomations: false }, expected: false }, + ]; + + for (const { label, body, expected } of cases) { + it(`${label}: the materialised value and the server decision are the same`, async () => { + const fromServer = await serverDecision(body); + const fromSchema = declared(body); + // Assert the agreement itself first — a mismatch reports both values. + expect({ declared: fromSchema, server: fromServer }) + .toEqual({ declared: expected, server: expected }); + }); + } + + it('validating before sending cannot change the outcome', async () => { + // The concrete harm #6704 names: a client that parses its request through + // the published schema and sends the PARSED object used to get the opposite + // behaviour from one that sent the same body unvalidated. Drive both paths + // through the server and require one answer. + const raw = {}; + const validated = ImportRequestSchema.parse({ format: 'json', rows: ROWS, ...raw }); + expect(await serverDecision(raw)).toBe(await serverDecision(validated)); + expect(await serverDecision(validated)).toBe(true); + }); + + it('the async job body agrees too — same schema object, separately published def', async () => { + // `POST /api/v1/data/:object/import/jobs` shares `prepareImportRequest` and + // its request def is an alias of the sync one, but both defs ship their own + // JSON Schema file, reference table and authorable-defaults row — so the + // async twin is asserted by name rather than inferred from the aliasing. + const fromSchema = CreateImportJobRequestSchema + .parse({ format: 'json', rows: ROWS }).runAutomations; + expect(fromSchema).toBe(await serverDecision({})); + expect(fromSchema).toBe(true); + }); + + it('an omitted flag is the only input whose declaration ever moved', async () => { + // Guards the reverse direction of the fix: the explicit spellings were + // already in agreement before #6704 and must not have been "fixed" into + // something else while the omitted case was corrected. + expect(declared({ runAutomations: false })).toBe(false); + expect(await serverDecision({ runAutomations: false })).toBe(false); + expect(declared({ runAutomations: true })).toBe(true); + expect(await serverDecision({ runAutomations: true })).toBe(true); + }); +}); diff --git a/packages/spec/authorable-defaults/api.json b/packages/spec/authorable-defaults/api.json index 06762c6b86..2b4a8b786a 100644 --- a/packages/spec/authorable-defaults/api.json +++ b/packages/spec/authorable-defaults/api.json @@ -47,7 +47,7 @@ "api/CreateFlowRequest:version = 1", "api/CreateImportJobRequest:createMissingOptions = false", "api/CreateImportJobRequest:dryRun = false", - "api/CreateImportJobRequest:runAutomations = false", + "api/CreateImportJobRequest:runAutomations = true", "api/CreateImportJobRequest:skipBlankMatchKey = false", "api/CreateImportJobRequest:treatAsHistorical = false", "api/CreateImportJobRequest:trimWhitespace = true", @@ -79,7 +79,7 @@ "api/GetPresignedUrlRequest:scope = \"user\"", "api/ImportRequest:createMissingOptions = false", "api/ImportRequest:dryRun = false", - "api/ImportRequest:runAutomations = false", + "api/ImportRequest:runAutomations = true", "api/ImportRequest:skipBlankMatchKey = false", "api/ImportRequest:treatAsHistorical = false", "api/ImportRequest:trimWhitespace = true", diff --git a/packages/spec/scripts/lib/default-changes.ts b/packages/spec/scripts/lib/default-changes.ts index 7801751f47..149c2da4fb 100644 --- a/packages/spec/scripts/lib/default-changes.ts +++ b/packages/spec/scripts/lib/default-changes.ts @@ -62,10 +62,54 @@ import type { DeclaredDefaultChange } from './authorable-defaults.js'; * * The table landed EMPTY at major 17 — that emptiness was the ratchet's own * proof that every default in the tree matched its recorded fingerprint. The - * entry below is the first one written (#6361), and it is worth noting what - * kind of change opened the account: not a behaviour flip, but the removal of a - * default that had never once been applied. + * first entry written (#6361) is worth noting for what kind of change opened + * the account: not a behaviour flip, but the removal of a default that had + * never once been applied. + * + * The `runAutomations` pair (#6704) is the same family seen from the other + * side, and the two read best together: where #6361 deleted a fiction, this one + * REPLACES a fiction with the fact. Both are declarations that no request path + * ever executed; neither moves a byte on the wire. A ratchet row whose `reason` + * says "nothing deployed changes behaviour" is therefore not a smell here — for + * an HTTP request schema nothing parses, it is the expected shape, and the + * consumer who IS affected is the one outside this repo who parses the + * published JSON Schema himself. + */ +/** + * Shared by the two `runAutomations` rows below — `CreateImportJobRequestSchema` + * IS `ImportRequestSchema`, so one edit moves two published defaults and the two + * rows would otherwise be a copy-paste pair that can drift apart. */ +const IMPORT_RUN_AUTOMATIONS_REASON = + 'The declared default was WRONG about the shipped server, and this row corrects the ' + + 'declaration rather than the behaviour. `POST /api/v1/data/:object/import` (and its ' + + 'async twin `.../import/jobs`) decides in `packages/rest/src/import-prepare.ts` with ' + + '`body?.runAutomations !== false` — an OMITTED flag has run automations since #2922, ' + + 'because automations always ran on import historically (the engine ignored the flag ' + + 'entirely before then) and because the platform convention is to fire triggers on ' + + 'import (Salesforce does). The schema said the opposite, in both machine-readable and ' + + 'human-readable form: `.default(false)` shipped in `@objectstack/spec`\'s JSON Schema, ' + + 'and the `describe` prose ("off by default for bulk") rendered into the published ' + + 'reference tables for BOTH defs. ' + + 'Nothing in this repo reconciled the two, because no request path parses an import ' + + 'body through this schema — the route reads the raw body, and the only reference to ' + + '`CreateImportJobRequestSchema` is the declarative `ImportJobApiContracts` catalog ' + + 'entry, which is a declaration and not a parse. So NO deployed caller changes ' + + 'behaviour here: a request that omitted the key ran automations before and runs them ' + + 'after. ' + + 'The consumer who WAS affected — and who is the reason this is a correction and not a ' + + 'cosmetic edit — lives outside this repo: a client or SDK that parses its request ' + + 'through the published schema materialised `runAutomations: false` from the declared ' + + 'default and SENT it explicitly, and the server honoured that. Identical request ' + + 'bodies therefore produced opposite behaviour depending on whether the caller ' + + 'validated before sending, with the validating caller silently losing its triggers. ' + + 'To keep automations OFF, write it — `runAutomations: false` — which was always the ' + + 'only spelling the server actually read. To keep what the server actually did for ' + + 'you, change nothing. Reading a materialised `ImportRequestParsed.runAutomations` now ' + + 'yields `true` where it yielded `false`; the value it yields is now the value the ' + + 'server would have applied anyway. Maintainer ruling 2026-08-09 (#6704, disposition ' + + 'A: the spec follows the runtime).'; + export const DEFAULT_CHANGES_BY_MAJOR: Readonly> = { 17: [ { @@ -93,5 +137,21 @@ export const DEFAULT_CHANGES_BY_MAJOR: Readonly { }); }); }); + +// ========================================== +// Import Request — runAutomations declared default (#6704) +// ========================================== + +/** + * The DECLARED default of `runAutomations`, pinned against the value the server + * actually applies. + * + * This half of the pin is deliberately schema-local: it asserts what a consumer + * who validates a request body through the published schema MATERIALISES. The + * other half — that the materialised value is the same one + * `POST /data/:object/import` decides on — lives in + * `packages/rest/src/import-run-automations-agreement.test.ts`, because only + * that package can reach both the schema and `prepareImportRequest`. Neither + * half alone is the fact #6704 is about: the fact is the AGREEMENT. + * + * Before #6704 the two disagreed on exactly one input — the omitted key — and + * that is the case a reader should look at first. + */ +describe('ImportRequestSchema — runAutomations declared default (#6704)', () => { + const bodyWithout = { format: 'json' as const, rows: [{ title: 'a' }] }; + + it('materialises `true` when the caller omits the key', () => { + expect(ImportRequestSchema.parse(bodyWithout).runAutomations).toBe(true); + }); + + it('keeps an explicit opt-out — `false` survives the parse', () => { + expect(ImportRequestSchema.parse({ ...bodyWithout, runAutomations: false }).runAutomations) + .toBe(false); + }); + + it('keeps an explicit opt-in', () => { + expect(ImportRequestSchema.parse({ ...bodyWithout, runAutomations: true }).runAutomations) + .toBe(true); + }); + + it('says the same thing on the async job body — it is the same schema object', () => { + // `CreateImportJobRequestSchema === ImportRequestSchema`, but both defs are + // PUBLISHED separately (two JSON Schema files, two reference tables, two + // authorable-defaults rows), so the async twin is asserted by name rather + // than left to the reader to infer from the aliasing. + expect(CreateImportJobRequestSchema.parse(bodyWithout).runAutomations).toBe(true); + expect( + CreateImportJobRequestSchema.parse({ ...bodyWithout, runAutomations: false }).runAutomations, + ).toBe(false); + }); + + it('describes the default it declares — the prose ships in the reference tables', () => { + // The old prose ("off by default for bulk") rendered into + // `content/docs/references/api/export.mdx` for BOTH defs and told an author + // the opposite of what the server does. Pin the direction, not the wording. + const described = (ImportRequestSchema.shape.runAutomations as { description?: string }) + .description ?? ''; + expect(described).not.toMatch(/off by default/i); + expect(described).toMatch(/ON by default/); + }); +}); diff --git a/packages/spec/src/api/export.zod.ts b/packages/spec/src/api/export.zod.ts index 2c8c073464..2ec32e830c 100644 --- a/packages/spec/src/api/export.zod.ts +++ b/packages/spec/src/api/export.zod.ts @@ -340,8 +340,15 @@ export const ImportRequestSchema = lazySchema(() => z.object({ .describe('insert / update / upsert semantics'), matchFields: z.array(z.string()).optional() .describe('Fields that identify an existing record (required for update/upsert)'), - runAutomations: z.boolean().default(false) - .describe('Fire triggers/hooks for each imported row (off by default for bulk)'), + runAutomations: z.boolean().default(true) + .describe( + 'Fire triggers/hooks for each imported row. ON by default, and opting out must be ' + + 'explicit: automations always ran on import historically (the engine ignored this flag ' + + 'until #2922), so a caller that wants a silent bulk load sends `runAutomations: false` — ' + + 'omitting the key runs them. This matches platform convention (Salesforce fires triggers ' + + 'on import by default). One boundary: a `dryRun` preview runs NO automations whatever ' + + 'this flag says (#6037).', + ), treatAsHistorical: z.boolean().default(false) .describe('Import as established historical facts. Two effects, both off by default so a normal import is unchanged: (1) skip the state_machine rule so mid-lifecycle rows (e.g. already-closed tickets, closed_won deals) are not rejected by initialStates (#3479); and (2) preserve the original audit timeline — keep the supplied created_at / updated_at / updated_by and author-declared business readonly fields (e.g. closed_at, resolved_by) instead of stamping-now / stripping them (#3493). Undoing a historical import mirrors (2): the captured pre-import values are restored verbatim rather than re-stamped (#3556).'), trimWhitespace: z.boolean().default(true) diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 9bbc73981e..3cebe156df 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -2980,6 +2980,70 @@ const step17: MigrationStep = { + 'reports the total across the whole matching inbox rather than the window. A caller ' + 'that omitted `limit` receives the same 50 rows it always received.', }, + { + id: 'import-run-automations-declared-default-corrected', + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a + // code span AND a table cell (see the note on `spec-type-alias-input-suffix-retired`). + surface: + 'api.ImportRequest runAutomations — the declared default of the key on BOTH import ' + + 'bodies, POST /api/v1/data/:object/import (ImportRequest) and its async twin POST ' + + '/api/v1/data/:object/import/jobs (CreateImportJobRequest, which IS the same schema ' + + 'object). It was declared default(false) and described as "off by default for ' + + 'bulk"; it is now default(true), which is what the server has always done', + replacement: + 'an explicit runAutomations: false on any import request that is meant to load rows ' + + 'without firing triggers/hooks. That spelling is unchanged and has always been the ' + + 'only one the server read — what changes is that omitting the key now DECLARES what ' + + 'it already DID. Callers who want automations on need write nothing', + reason: + 'A DECLARATION corrected to match a runtime that did not move — the inverse of a ' + + "behaviour flip, and registered here for the reason protocol 12's " + + '`rest-requireauth-default-flip` and this major\'s ' + + '`action-descriptor-resume-authority-default-flip` are: whether a given import was ' + + 'meant to fire triggers is a judgment no transform can make, so the prescription is ' + + 'a TODO rather than a rewrite. The server decides in import-prepare.ts with ' + + '`body?.runAutomations !== false`, i.e. an omitted flag runs automations, and has ' + + 'since #2922 — automations always ran on import historically (the engine ignored ' + + 'the flag entirely before then), so opt-out was made the explicit act, matching ' + + 'platform convention. The schema said the opposite in both machine-readable and ' + + "human-readable form, and both SHIPPED: `.default(false)` in `@objectstack/spec`'s " + + 'JSON Schema, and the describe prose in the published reference tables for both ' + + 'defs. ' + + '⚠️ Nothing in this repo reconciled the two and NO deployed caller changes ' + + 'behaviour: no request path parses an import body through this schema — the route ' + + 'reads the raw body, and the sole reference to `CreateImportJobRequestSchema` is ' + + 'the declarative `ImportJobApiContracts` catalog entry, a declaration and not a ' + + 'parse. That is exactly why this needed a ruling rather than a docs edit: the ' + + 'divergence was unobservable in-tree and observable only to a consumer OUTSIDE it. ' + + 'A client or SDK that validated its request through the published schema ' + + 'materialised `runAutomations: false` from the declared default and sent it ' + + 'explicitly, and the server honoured it — so the same request body produced ' + + 'opposite behaviour depending on whether the caller validated before sending, with ' + + 'the validating caller silently losing its triggers. Nothing rejected it, nothing ' + + 'warned, and the reference page told an author the wrong thing in the other ' + + 'direction. There is deliberately NO schema tombstone and no D2 conversion: no key ' + + 'is removed, and an HTTP request body is neither authored nor persisted — the same ' + + 'disposition `notification-list-cursor-retired` (#6361) takes for the sibling ' + + 'default on this major, and `batch-options-validate-only-retired` before it. The ' + + 'declared move itself is recorded mechanically, per key, in ' + + 'DEFAULT_CHANGES_BY_MAJOR[17] (#4666), whose `from`/`to` fingerprints are ' + + 're-derived on every build. Maintainer ruling 2026-08-09 (#6704, disposition A: ' + + 'the spec follows the runtime). ADR-0049 / ADR-0078.', + acceptanceCriteria: + 'Every import request of yours that must NOT fire triggers sends `runAutomations: ' + + 'false` explicitly, rather than omitting the key and trusting the old declared ' + + 'default. The check is worth doing precisely where it looks unnecessary: if you ' + + 'build the body by parsing it through `ImportRequestSchema` (or the published JSON ' + + 'Schema) and then send the PARSED object, your bulk loads were running with ' + + 'automations OFF and will now run with them ON — that is the only class whose ' + + 'behaviour changes, and it changes toward what an unvalidated caller always got. ' + + '⚠️ Behaviour on the wire is deliberately UNCHANGED and should be verified as ' + + 'such: a body that omits `runAutomations` fired triggers before this change and ' + + 'fires them after, and `runAutomations: false` turns them off before and after. ' + + 'Nothing starts being refused — the route never validated this body against the ' + + 'schema and does not begin to. `dryRun` is unaffected and still runs NO automations ' + + 'whatever the flag says (#6037).', + }, ], };