Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changeset/engine-update-upsert-retired.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
"@objectstack/spec": major
"@objectstack/objectql": patch
---

refactor(spec)!: remove the never-implemented `upsert` flag from `engine.update()`'s option surface (#8057, ADR-0049 enforce-or-remove)

`options.upsert` was declared on both update-options schemas
(`EngineUpdateOptionsSchema` and the deprecated `DataEngineUpdateOptionsSchema`)
and sat on the engine's update allowlist, yet **no engine or driver path ever
read it**: it was not a driver pass-through key and `ObjectQL.update()` never
referenced it. A caller passing `{ upsert: true }` got silence — not a refusal,
not an upsert. The one place a caller would learn the truth was by reading the
engine, and the strict-unknown gate (the mechanism that normally catches a
meaningless option) actively vouched for the key.

FROM → TO: delete `upsert` from any `engine.update()` / `updateData` option
bag. For create-if-absent intent, express it explicitly — read the row first
(`findOne`) and call `insert` or `update` on what you find. Note the by-id
update branch now throws `RECORD_NOT_FOUND` when the id names no row (#7867's
not-found gate); a future first-class upsert must reconcile with that gate by
design, which is why the flag is removed rather than implemented.

The retirement kit:

- `retiredKey()` tombstones on BOTH schemas, one shared prescription
(`ENGINE_UPDATE_UPSERT_REMOVED`): authoring the key is a tsc error and a
parse error carrying the fix.
- The objectql engine drops `upsert` from `ENGINE_UPDATE_OPTION_KEYS` and
quotes the same prescription from its unknown-option gate
(`ENGINE_RETIRED_OPTION_MESSAGES`), so the untyped runtime path is loud too.
- **ADR-0087 registry**: both keys registered in `RETIRED_KEYS_BY_MAJOR[17]`
plus the D3 semantic entry `engine-update-upsert-retired`. **No D2
conversion**, deliberately: an engine option bag is call-time only — nobody
authors one and nothing persists one (the `BatchOptions.validateOnly`
disposition).
- Baselines (`authorable-surface/data.json` `[RETIRED]` marks,
`authorable-defaults`, `api-surface`, `spec-changes.json`, upgrade guide,
reference docs) regenerated deliberately.

No runtime behaviour changes for any in-tree caller — zero production call
sites passed the flag (measured in #8057); the only references were the spec's
own schema tests, now re-pointed to assert the refusal.

<!-- adr-0087: registered engine-update-upsert-retired -->
8 changes: 4 additions & 4 deletions content/docs/references/data/data-engine.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -291,7 +291,7 @@ This schema accepts one of the following structures:
| **object** | `string` | ✅ | |
| **data** | `Record<string, any>` | ✅ | |
| **id** | `string \| number` | optional | ID for single update, or use where in options |
| **options** | `{ context?: object; where?: Record<string, any> \| any; upsert?: boolean; multi?: boolean; … }` | optional | |
| **options** | `{ context?: object; where?: Record<string, any> \| any; multi?: boolean; returning?: boolean; … }` | optional | |

---

Expand DownExpand Up@@ -402,7 +402,7 @@ Options for DataEngine.update operations
| :--- | :--- | :--- | :--- |
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |
| **upsert** | `boolean` | optional | |
| **upsert** | `never` | optional | [REMOVED] `update.options.upsert` was removed in @objectstack/spec 17 (#8057, ADR-0049) — it was declared and allowlisted but never implemented: no engine or driver path ever read it, so `{ upsert: true }` was accepted and silently dropped and the update stayed a plain update. Delete the key. Express create-if-absent explicitly: a by-id update whose id names no row throws RECORD_NOT_FOUND (#7867's not-found gate) rather than inserting, so read the row first (`findOne`) and call `insert` or `update` on what you find. A first-class upsert, if ever built, must reconcile with that gate by design rather than through this silent flag. |
| **multi** | `boolean` | optional | |
| **returning** | `boolean` | optional | |

Expand All@@ -419,7 +419,7 @@ Options for DataEngine.update operations
| **object** | `string` | ✅ | |
| **data** | `Record<string, any>` | ✅ | |
| **id** | `string \| number` | optional | ID for single update, or use where in options |
| **options** | `{ context?: object; where?: Record<string, any> \| any; upsert?: boolean; multi?: boolean; … }` | optional | |
| **options** | `{ context?: object; where?: Record<string, any> \| any; multi?: boolean; returning?: boolean; … }` | optional | |


---
Expand DownExpand Up@@ -537,7 +537,7 @@ QueryAST-aligned options for DataEngine.update operations
| :--- | :--- | :--- | :--- |
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
| **where** | `Record<string, any> \| any` | optional | |
| **upsert** | `boolean` | optional | |
| **upsert** | `never` | optional | [REMOVED] `update.options.upsert` was removed in @objectstack/spec 17 (#8057, ADR-0049) — it was declared and allowlisted but never implemented: no engine or driver path ever read it, so `{ upsert: true }` was accepted and silently dropped and the update stayed a plain update. Delete the key. Express create-if-absent explicitly: a by-id update whose id names no row throws RECORD_NOT_FOUND (#7867's not-found gate) rather than inserting, so read the row first (`findOne`) and call `insert` or `update` on what you find. A first-class upsert, if ever built, must reconcile with that gate by design rather than through this silent flag. |
| **multi** | `boolean` | optional | |
| **returning** | `boolean` | optional | |

Expand Down
3 changes: 3 additions & 0 deletions docs/protocol-upgrade-guide.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -413,6 +413,9 @@ This is a CODE-path API, not stored metadata, so — like `hook-register-empty-o

ONE AUTHOR-REACHABLE SURFACE reaches this indirectly and is why it is not purely a code-side note: a saved report's `query.orderBy` (`sys_saved_report`) is forwarded verbatim into `engine.find` by `plugin-reports`, bypassing the ingress gate. A report authored to sort by a formula field used to run and return rows in an arbitrary order; it now fails loudly, with the remedy in the message. One further path is deliberately NOT a refusal: a nested `expand` sort raises this refusal inside `expandRelatedRecords`, whose pre-existing graceful-degradation `catch` swallows every expand failure and retains the raw foreign keys — so that path moves from silent to OBSERVABLE (a warning naming the field and the fix) rather than refusing. Reversing that backstop is a separate decision on all expand failure modes. #7095, #6994, #6924, #4226, #4256, #3821, ADR-0112.
- Done when: No `engine.find` / `engine.findOne` call site sorts by a `formula` field, and no saved report's `query.orderBy` names one — grep your report definitions for an `orderBy` field whose object declares it as a `formula`, and denormalise it onto a stored column written when the source changes. A `summary` / rollup field needs no action: it has a real maintained column and sorts correctly. Reads complete with no `INVALID_SORT` naming a formula field, and no "Failed to expand relationship field" warning whose error text names one.
- **`engine-update-upsert-retired`** — `data.engine.update options.upsert` → (removed — never implemented; express create-if-absent explicitly: `findOne` first, then `insert` or `update` on what you find)
- Why not automatic: The `upsert` flag promised insert-if-absent on `engine.update()` but no engine or driver path ever read it: the key was declared on both update-options schemas and allowlisted by the unknown-option gate, yet `ObjectQL.update()` never referenced it and it was not a driver pass-through key — `{ upsert: true }` was accepted and silently dropped and the update stayed a plain update (ADR-0049 declared-but-unenforced). There is no behaviour to preserve and nothing stored to rewrite (it only ever appeared in a call-time option bag). Any future first-class upsert must reconcile with #7867's not-found gate — a by-id update whose id names no row throws RECORD_NOT_FOUND rather than inserting — which is why the flag is removed rather than implemented here.
- Done when: No caller passes `options.upsert` to `engine.update()`; a call that includes it is refused loudly (the engine gate and both schemas quote the #8057 prescription) instead of succeeding with the option silently ignored.
- **`enhanced-api-error-field-errors-renamed`** — `api.enhancedApiError.fieldErrors` → fields
- Why not automatic: The wire has always carried `fields` — the validators, import coercion, validation-failure.ts, @objectstack/client and the console's field-error extractor all say `fields`, and nothing ever emitted `fieldErrors`, so a reader keying on it was reading a field no server sent (ADR-0078's silently-inert declaration, on the error envelope). This is a RESPONSE surface: no stack, example or template carries the key, so there is no source for the chain to rewrite — the schema tombstones it via retiredKey() and consumers move their read themselves. ADR-0114 D4, #3977.
- Done when: No consumer reads `error.fieldErrors`; per-field validation detail is read from `error.fields`, and constructing an EnhancedApiError with `fieldErrors` fails to parse with the rename prescription instead of silently losing the array.
Expand Down
16 changes: 15 additions & 1 deletion packages/objectql/src/engine-unknown-option.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -142,6 +142,20 @@ describe('unknown engine option keys are rejected (#4371 option 2)', () => {
.rejects.toThrow(/#4286, ADR-0049/);
});

it('update `upsert` is rejected with its #8057 tombstone — it was accepted and silently dropped', async () => {
// [#8057] `upsert` sat on the update allowlist while nothing read it
// (declared-but-unenforced, ADR-0049). The refusal quotes the spec's
// prescription: delete the key; create-if-absent is explicit now that
// the by-id branch throws on a missing row (#7867's not-found gate).
await expect(engine.update('task', { title: 'Z' }, { where: { id: a.id }, upsert: true } as any))
.rejects.toThrow(/`update\.options\.upsert` was removed .*#8057, ADR-0049.*never implemented.*Delete the key.*not-found gate/s);
});

it('a null-valued `upsert` stays a withdrawal — no intent a drop could lose', async () => {
const updated = await engine.update('task', { title: 'A4' }, { where: { id: a.id }, upsert: null } as any);
expect(updated).toBeDefined();
});

// ── null stays a withdrawal ─────────────────────────────────────────

it('a null-valued unknown key is a withdrawal, not a rejection', async () => {
Expand DownExpand Up@@ -203,7 +217,7 @@ describe('unknown engine option keys are rejected (#4371 option 2)', () => {
// ── drift pin: legal sets stay glued to the spec schemas ────────────

it('each legal set covers its schema shape (minus tombstones) and only the documented extras', () => {
const TOMBSTONES = new Set(['cursor', 'distinct']);
const TOMBSTONES = new Set(['cursor', 'distinct', 'upsert']);
const PASSTHROUGH = ['transaction', 'tenantId', 'tenantIds', 'timezone', 'bypassTenantAudit', 'preserveAudit'];
const expectSetMatches = (
setName: string,
Expand Down
15 changes: 11 additions & 4 deletions packages/objectql/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ import {
foldQueryAliasSlots,
QUERY_CURSOR_REMOVED,
QUERY_DISTINCT_REMOVED,
ENGINE_UPDATE_UPSERT_REMOVED,
type QueryAliasSlot,
type DroppedFieldsEvent
} from '@objectstack/spec/data';
Expand DownExpand Up@@ -328,9 +329,9 @@ const ENGINE_DRIVER_PASSTHROUGH_KEYS = [
* wire-alias rejection above.
*
* Sources, in order: the method's `Engine*OptionsSchema` declared keys (minus
* the `retiredKey` tombstones `cursor`/`distinct`, which get their tombstone
* quoted instead of a generic rejection — the schema keeps them ONLY to carry
* that message, and this runtime path never parses); `searchFields` (read by
* the `retiredKey` tombstones `cursor`/`distinct`/`upsert` (#8057), which get
* their tombstone quoted instead of a generic rejection — the schema keeps them
* ONLY to carry that message, and this runtime path never parses); `searchFields` (read by
* `find` at the `$search` expansion, sent by the protocol layer);
* `onFieldsDropped` and `strictReadonlyWrites` (`WriteObservabilityOptions` —
* contract-declared, deliberately outside the serializable Zod schema: the
Expand All@@ -350,7 +351,7 @@ const ENGINE_FIND_OPTION_KEYS: ReadonlySet<string> = new Set([
...ENGINE_DRIVER_PASSTHROUGH_KEYS,
]);
const ENGINE_UPDATE_OPTION_KEYS: ReadonlySet<string> = new Set([
'context', 'where', 'upsert', 'multi', 'returning', 'onFieldsDropped', 'strictReadonlyWrites',
'context', 'where', 'multi', 'returning', 'onFieldsDropped', 'strictReadonlyWrites',
...ENGINE_DRIVER_PASSTHROUGH_KEYS,
]);
const ENGINE_DELETE_OPTION_KEYS: ReadonlySet<string> = new Set([
Expand DownExpand Up@@ -457,6 +458,12 @@ function readStoredAutonumberCounter(value: string, prefix: string, suffix: stri
const ENGINE_RETIRED_OPTION_MESSAGES: Record<string, string> = {
cursor: QUERY_CURSOR_REMOVED,
distinct: QUERY_DISTINCT_REMOVED,
// [#8057] `update.options.upsert` — declared-but-unenforced (ADR-0049): the
// key sat on the update allowlist while nothing read it, so `{ upsert: true }`
// was accepted and silently dropped. Removed rather than implemented; the
// spec's tombstone carries the create-if-absent prescription (#7867's
// not-found gate is the semantics a caller must reconcile with).
upsert: ENGINE_UPDATE_UPSERT_REMOVED,
};

/**
Expand Down
1 change: 1 addition & 0 deletions packages/spec/api-surface/data.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,6 +202,7 @@
"DriverVocabularyEntry (interface)",
"DroppedFieldsEvent (type)",
"DroppedFieldsEventSchema (const)",
"ENGINE_UPDATE_UPSERT_REMOVED (const)",
"ESignatureConfig (type)",
"ESignatureConfigParsed (type)",
"ESignatureConfigSchema (const)",
Expand Down
2 changes: 0 additions & 2 deletions packages/spec/authorable-defaults/data.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,7 +19,6 @@
"data/DataEngineInsertOptions:returning = true",
"data/DataEngineUpdateOptions:multi = false",
"data/DataEngineUpdateOptions:returning = false",
"data/DataEngineUpdateOptions:upsert = false",
"data/DataEngineVectorFindRequest:limit = 5",
"data/Datasource:active = true",
"data/Datasource:autoConnect = false",
Expand All@@ -33,7 +32,6 @@
"data/EngineDeleteOptions:multi = false",
"data/EngineUpdateOptions:multi = false",
"data/EngineUpdateOptions:returning = false",
"data/EngineUpdateOptions:upsert = false",
"data/ExternalColumn:primaryKey = false",
"data/ExternalDatasourceSettings:allowWrites = false",
"data/ExternalDatasourceSettings:queryTimeoutMs = 30000",
Expand Down
4 changes: 2 additions & 2 deletions packages/spec/authorable-surface/data.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -142,7 +142,7 @@
"data/DataEngineUpdateOptions:filter",
"data/DataEngineUpdateOptions:multi",
"data/DataEngineUpdateOptions:returning",
"data/DataEngineUpdateOptions:upsert",
"data/DataEngineUpdateOptions:upsert [RETIRED]",
"data/DataEngineUpdateRequest:data",
"data/DataEngineUpdateRequest:id",
"data/DataEngineUpdateRequest:method",
Expand DownExpand Up@@ -304,7 +304,7 @@
"data/EngineUpdateOptions:context",
"data/EngineUpdateOptions:multi",
"data/EngineUpdateOptions:returning",
"data/EngineUpdateOptions:upsert",
"data/EngineUpdateOptions:upsert [RETIRED]",
"data/EngineUpdateOptions:where",
"data/EqualityOperator:$eq",
"data/EqualityOperator:$ne",
Expand Down
1 change: 1 addition & 0 deletions packages/spec/export-origins/data.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,6 +202,7 @@
"DriverVocabularyEntry": "src/data/driver/config-registry.zod.ts#DriverVocabularyEntry (interface)",
"DroppedFieldsEvent": "src/data/data-engine.zod.ts#DroppedFieldsEvent (type)",
"DroppedFieldsEventSchema": "src/data/data-engine.zod.ts#DroppedFieldsEventSchema (const)",
"ENGINE_UPDATE_UPSERT_REMOVED": "src/data/data-engine.zod.ts#ENGINE_UPDATE_UPSERT_REMOVED (const)",
"ESignatureConfig": "src/data/document.zod.ts#ESignatureConfig (type)",
"ESignatureConfigParsed": "src/data/document.zod.ts#ESignatureConfigParsed (type)",
"ESignatureConfigSchema": "src/data/document.zod.ts#ESignatureConfigSchema (const)",
Expand Down
Loading
Loading