From a00813bd73493bfd2b5e5872c6845b0de4a66917 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 05:58:45 +0000 Subject: [PATCH 1/2] feat(spec): declare the not-found arm on IDataDriver.update() and un-mask driver-memory's published update/upsert types (#13878) IDataDriver.update() gains '| null' with a docblock that says when it is returned, reusing findOne's shape and delete's not-found vocabulary. InMemoryDriver.update()/upsert() carry explicit return types so the published .d.ts stops reading Promise; upsert asserts the arm it can never take instead of widening its door. A type-level pin holds both the contract and the driver; the landed behaviour pin is unchanged. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- ...river-memory-update-upsert-honest-types.md | 5 ++ .../idatadriver-update-declares-null.md | 5 ++ .../metadata-database-loader-update-null.md | 5 ++ .../driver-memory/src/memory-driver.test.ts | 10 ++- .../driver-memory/src/memory-driver.ts | 23 ++++- .../src/memory-update-declared-null.test.ts | 88 +++++++++++++++++++ .../metadata/src/loaders/database-loader.ts | 4 +- packages/spec/src/contracts/data-driver.ts | 7 +- 8 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 .changeset/driver-memory-update-upsert-honest-types.md create mode 100644 .changeset/idatadriver-update-declares-null.md create mode 100644 .changeset/metadata-database-loader-update-null.md create mode 100644 packages/drivers/driver-memory/src/memory-update-declared-null.test.ts diff --git a/.changeset/driver-memory-update-upsert-honest-types.md b/.changeset/driver-memory-update-upsert-honest-types.md new file mode 100644 index 0000000000..10b9d62bc5 --- /dev/null +++ b/.changeset/driver-memory-update-upsert-honest-types.md @@ -0,0 +1,5 @@ +--- +'@objectstack/driver-memory': minor +--- + +`InMemoryDriver.update()` and `upsert()` publish their honest types. The emitted `.d.ts` read `Promise` for both — the return type was inferred through the backing store's `any[]` rows, so the union collapsed and no caller was asked to narrow. They are now declared as the contract declares them: `update()` returns `Promise | null>` (the `null` arm is the non-`strictMode` miss the driver has always answered with), `upsert()` returns `Promise>`. No runtime behaviour changes. diff --git a/.changeset/idatadriver-update-declares-null.md b/.changeset/idatadriver-update-declares-null.md new file mode 100644 index 0000000000..9e17a5baf8 --- /dev/null +++ b/.changeset/idatadriver-update-declares-null.md @@ -0,0 +1,5 @@ +--- +'@objectstack/spec': minor +--- + +`IDataDriver.update()` now declares its not-found arm: the return type is `Promise | null>`, and the docblock says when `null` is returned (no record with that id, on a driver not configured to throw on missing records). This is the shape `findOne()` already carries and the not-found vocabulary `delete()` already uses; it declares the behaviour four of the six shipped drivers have always had, which the previous declaration forbade. Callers that read fields off an `update()` result now narrow the `null` arm first — a compile-time obligation in place of a silent runtime hazard. diff --git a/.changeset/metadata-database-loader-update-null.md b/.changeset/metadata-database-loader-update-null.md new file mode 100644 index 0000000000..e82f71e7e4 --- /dev/null +++ b/.changeset/metadata-database-loader-update-null.md @@ -0,0 +1,5 @@ +--- +'@objectstack/metadata': patch +--- + +`DatabaseLoader`'s private driver-path update helper is typed with the `null` arm `IDataDriver.update()` now declares; both of its callers discard the result. No runtime behaviour changes. diff --git a/packages/drivers/driver-memory/src/memory-driver.test.ts b/packages/drivers/driver-memory/src/memory-driver.test.ts index e29eaec97b..e189a1e000 100644 --- a/packages/drivers/driver-memory/src/memory-driver.test.ts +++ b/packages/drivers/driver-memory/src/memory-driver.test.ts @@ -67,7 +67,10 @@ describe('InMemoryDriver', () => { { active: false } ); - expect(updateResult.active).toBe(false); + // The declared type carries the not-found arm (#13878); a caller reading + // fields narrows first. The row exists, so the arm is not taken here. + expect(updateResult).not.toBeNull(); + expect(updateResult!.active).toBe(false); const results = await driver.find(testTable, { fields: ['active'] }); expect(results[0].active).toBe(false); @@ -98,8 +101,9 @@ describe('InMemoryDriver', () => { const originalCreatedAt = created.created_at; const updated = await driver.update(testTable, '1', { name: 'Alice Updated' }); - expect(updated.created_at).toBe(originalCreatedAt); - expect(updated.name).toBe('Alice Updated'); + expect(updated).not.toBeNull(); + expect(updated!.created_at).toBe(originalCreatedAt); + expect(updated!.name).toBe('Alice Updated'); }); }); diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index c5397c9ac9..bbc35279aa 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -666,7 +666,15 @@ export class InMemoryDriver implements IDataDriver { return { ...newRecord }; } - async update(object: string, id: string | number, data: Record, options?: DriverOptions) { + /** + * Declared as the contract declares it (#13878): the `null` arm is the + * non-`strictMode` miss the behaviour pin has always held, and the explicit + * annotation is what keeps that arm visible to `tsc` — left to inference, + * the return type collapses to `any` through the backing store's + * `any[]` rows and the union is swallowed, so the published `.d.ts` read + * `Promise` and no caller was ever asked to narrow. + */ + async update(object: string, id: string | number, data: Record, options?: DriverOptions): Promise | null> { this.logger.debug('Update operation', { object, id }); const table = this.getTable(object); @@ -698,7 +706,7 @@ export class InMemoryDriver implements IDataDriver { return { ...updatedRecord }; } - async upsert(object: string, data: Record, conflictKeys?: string[], options?: DriverOptions) { + async upsert(object: string, data: Record, conflictKeys?: string[], options?: DriverOptions): Promise> { this.logger.debug('Upsert operation', { object, conflictKeys }); const table = this.getTable(object); @@ -712,7 +720,16 @@ export class InMemoryDriver implements IDataDriver { if (existingRecord) { this.logger.debug('Record exists, updating', { object, id: existingRecord.id }); - return this.update(object, existingRecord.id, data, options); + const updated = await this.update(object, existingRecord.id, data, options); + // `existingRecord` was read from this same table above and nothing + // yields in between, so `update` cannot miss here: the `null` arm of + // its declared type is unreachable on this path. An upsert never + // answers "not found" — say so loudly rather than widen this door's + // declared return to carry an arm it can never produce (#13878). + if (updated === null) { + throw new Error(`Record with ID ${existingRecord.id} vanished from ${object} during upsert`); + } + return updated; } else { this.logger.debug('Record does not exist, creating', { object }); return this.create(object, data, options); diff --git a/packages/drivers/driver-memory/src/memory-update-declared-null.test.ts b/packages/drivers/driver-memory/src/memory-update-declared-null.test.ts new file mode 100644 index 0000000000..78e59088d8 --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-update-declared-null.test.ts @@ -0,0 +1,88 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #13878 — the declared return type of `update()` carries its not-found arm, +// and the driver's published type is the contract's, not `any`. +// +// `InMemoryDriver.update()` has always answered a missing id with `null` when +// `strictMode` is off (the behaviour pin in `memory-driver.test.ts` holds it), +// while `IDataDriver.update()` declared `Promise>` — +// a contract violated by 4 of 6 shipped drivers and never seen by `tsc`, +// because the driver's return type was INFERRED through the backing store's +// `any[]` rows: the union collapsed to `any`, the published `.d.ts` read +// `Promise`, and no caller was ever asked to narrow. The maintainer's +// ruling (2026-09-01) declares the arm — `Promise | +// null>`, the shape `findOne` already carries — and un-masks the driver. +// +// This file pins BOTH halves at the type level, inside the package's tsc +// program (`tsconfig.json` selects `src/**/*`, tests included): +// +// 1. the CONTRACT: `IDataDriver.update()` resolves to +// `Record | null` — read through `@objectstack/spec`'s +// built `.d.ts`, so reverting the declaration alone reds this file; +// 2. the DRIVER: `InMemoryDriver.update()` / `upsert()` are not `any` and +// resolve to exactly the contract's types — reverting the driver's +// explicit annotations alone reds this file too (the `any` mask +// returns and `IsAny` flips). +// +// The `satisfies`-free typed-const form is the one the sibling drivers use +// (`turso-driver-options-door.test.ts`, `sql-driver-distinct-filter- +// narrowing.test.ts`); the runtime cases below make the consts observable so +// the file is a test and not a declaration. + +import { describe, it, expect } from 'vitest'; +import { InMemoryDriver } from './memory-driver.js'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +/** `any` defeats ordinary assignability checks; this is the standard detector. */ +type IsAny = 0 extends 1 & T ? true : false; +/** Exact (mutual, non-`any`) type equality. */ +type Equals = (() => T extends A ? 1 : 2) extends (() => T extends B ? 1 : 2) ? true : false; + +type ContractUpdate = Awaited>; +type MemoryUpdate = Awaited>; +type MemoryUpsert = Awaited>; + +// 1. The contract declares the not-found arm. +const contractUpdateDeclaresNull: Equals | null> = true; + +// 2. The driver's doors are un-masked and read exactly as the contract does. +const memoryUpdateIsAny: IsAny = false; +const memoryUpdateIsContract: Equals | null> = true; +const memoryUpsertIsAny: IsAny = false; +const memoryUpsertIsContract: Equals> = true; + +describe('InMemoryDriver.update()/upsert() declared return types (#13878)', () => { + it('pins the contract and the driver at the type level', () => { + expect([ + contractUpdateDeclaresNull, + memoryUpdateIsAny, + memoryUpdateIsContract, + memoryUpsertIsAny, + memoryUpsertIsContract, + ]).toEqual([true, false, true, false, true]); + }); + + it('update() on a missing id resolves to null, and the declared type makes the caller narrow', async () => { + const driver = new InMemoryDriver(); + await driver.connect(); + + const result = await driver.update('t', 'missing', { name: 'x' }); + expect(result).toBeNull(); + + // The narrowing the declared type now demands of every caller: a field + // read is only reachable behind the `null` check. + const name = result === null ? 'absent' : result.name; + expect(name).toBe('absent'); + }); + + it('upsert() over an existing id returns the merged record and never the not-found arm', async () => { + const driver = new InMemoryDriver(); + await driver.connect(); + await driver.create('t', { id: '1', name: 'before', keep: 'kept' }); + + const merged = await driver.upsert('t', { id: '1', name: 'after' }); + expect(merged.id).toBe('1'); + expect(merged.name).toBe('after'); + expect(merged.keep).toBe('kept'); + }); +}); diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index 9e1f2aa097..dcc400b85f 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -307,7 +307,9 @@ export class DatabaseLoader implements MetadataLoader { return this.driver!.create(table, data); } - private async _update(table: string, id: string, data: Record): Promise> { + // `null` is the driver path's not-found answer (`IDataDriver.update()`, + // #13878); both callers here resolve the row first and discard the result. + private async _update(table: string, id: string, data: Record): Promise | null> { if (this.engine) { // [#11231] The resolved id AFTER the spread, so it WINS — the same // convention as `rest-server.ts`'s batch arm and `protocol.updateData` diff --git a/packages/spec/src/contracts/data-driver.ts b/packages/spec/src/contracts/data-driver.ts index fd63a5a2d7..8bec163178 100644 --- a/packages/spec/src/contracts/data-driver.ts +++ b/packages/spec/src/contracts/data-driver.ts @@ -189,8 +189,13 @@ export interface IDataDriver { /** * Update an existing record by ID. * MUST return `id` as string. MUST NOT return implementation details like `_id`. + * @returns The updated record, or `null` if no record with that id exists — + * the same "addressed one row, which may not exist" shape as {@link findOne}, + * and the not-found vocabulary {@link delete} already carries (`false`). A + * driver configured to throw on missing records (`strictMode`) throws + * instead of returning `null`. */ - update(object: string, id: string | number, data: Record, options?: DriverOptions): Promise>; + update(object: string, id: string | number, data: Record, options?: DriverOptions): Promise | null>; /** * Upsert (Update or Insert) a record. From 7db94d680a71e05e18892851f8b50fc261ce48d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 07:02:07 +0000 Subject: [PATCH 2/2] fix(spec): mirror the not-found arm on DriverInterfaceSchema.update and carry BREAKING + ADR-0087 dispositions on the changesets (#13878 patch round) DriverInterfaceSchema.update outputs the same .nullable() record findOne already does, with the docblock sentence the TS interface carries. Both changesets keep minor and add the BREAKING banner plus the no-migration-prescription disposition; the driver-memory one names the unreachable-arm assertion in upsert(). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .changeset/driver-memory-update-upsert-honest-types.md | 8 +++++++- .changeset/idatadriver-update-declares-null.md | 8 +++++++- packages/spec/src/data/driver.zod.ts | 5 +++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/.changeset/driver-memory-update-upsert-honest-types.md b/.changeset/driver-memory-update-upsert-honest-types.md index 10b9d62bc5..c8066e5fd9 100644 --- a/.changeset/driver-memory-update-upsert-honest-types.md +++ b/.changeset/driver-memory-update-upsert-honest-types.md @@ -2,4 +2,10 @@ '@objectstack/driver-memory': minor --- -`InMemoryDriver.update()` and `upsert()` publish their honest types. The emitted `.d.ts` read `Promise` for both — the return type was inferred through the backing store's `any[]` rows, so the union collapsed and no caller was asked to narrow. They are now declared as the contract declares them: `update()` returns `Promise | null>` (the `null` arm is the non-`strictMode` miss the driver has always answered with), `upsert()` returns `Promise>`. No runtime behaviour changes. +feat(driver-memory): `update()` and `upsert()` publish their honest types (#13878) + +**BREAKING** for TypeScript consumers — a published TYPE-surface narrowing, the shape ADR-0087's 2026-08-30 addendum names (a published SDK method whose declared return moves off `any` onto the contract it always answered) — shipped as `minor` under the launch-window convention. The emitted `.d.ts` read `Promise` for both doors: the return types were inferred through the backing store's `any[]` rows, so the union collapsed and no caller was asked to narrow. They are now declared as the contract declares them — `update()` returns `Promise | null>` (the `null` arm is the non-`strictMode` miss the driver has always answered with), `upsert()` returns `Promise>`. A caller that read fields off `update()`'s result through the `any` now narrows the `null` arm first; a caller that leaned on `any` to read undeclared members of either result now types them. + +`upsert()` now asserts — throws on — the `null` arm of `update()` on a path it cannot reach (the row it updates was found in the same table a moment earlier, with nothing yielding in between), instead of widening its own declared return to carry an arm it can never produce. No reachable runtime behaviour changes. + + diff --git a/.changeset/idatadriver-update-declares-null.md b/.changeset/idatadriver-update-declares-null.md index 9e17a5baf8..9055dc47a5 100644 --- a/.changeset/idatadriver-update-declares-null.md +++ b/.changeset/idatadriver-update-declares-null.md @@ -2,4 +2,10 @@ '@objectstack/spec': minor --- -`IDataDriver.update()` now declares its not-found arm: the return type is `Promise | null>`, and the docblock says when `null` is returned (no record with that id, on a driver not configured to throw on missing records). This is the shape `findOne()` already carries and the not-found vocabulary `delete()` already uses; it declares the behaviour four of the six shipped drivers have always had, which the previous declaration forbade. Callers that read fields off an `update()` result now narrow the `null` arm first — a compile-time obligation in place of a silent runtime hazard. +feat(spec): `IDataDriver.update()` declares its not-found arm (#13878) + +**BREAKING** for TypeScript consumers, shipped as `minor` under the repo's launch-window convention for breaking changes (maintainer ruling 2026-09-01, option A). `IDataDriver.update()`'s return type is now `Promise | null>` — the shape `findOne()` already carries and the not-found vocabulary `delete()` already uses (`false`) — and its docblock says when `null` is returned: no record with that id exists, on a driver not configured to throw on missing records. The Zod mirror `DriverInterfaceSchema.update` carries the same `.nullable()` output and the same sentence. This declares the behaviour four of the six shipped drivers have always had, which the previous declaration forbade. + +What breaks: a caller that read fields straight off an `update()` result compiled before only because the arm was undeclared; it now narrows the `null` arm first — a compile-time obligation at the call site in place of a silent runtime hazard. No metadata key, no runtime behaviour and no wire shape moves. + + diff --git a/packages/spec/src/data/driver.zod.ts b/packages/spec/src/data/driver.zod.ts index 51b6eee585..1e452f531b 100644 --- a/packages/spec/src/data/driver.zod.ts +++ b/packages/spec/src/data/driver.zod.ts @@ -504,12 +504,13 @@ export const DriverInterfaceSchema = lazySchema(() => z.object({ * @param id - The unique identifier of the record. * @param data - The fields to update. * @param options - Driver options. - * @returns The updated record. + * @returns The updated record, or `null` if no record with that id exists + * (a driver configured to throw on missing records throws instead). * MUST return `id` as string. MUST NOT return implementation details like `_id`. */ update: z.function() .input(z.tuple([z.string(), z.string().or(z.number()), z.record(z.string(), z.unknown()), DriverOptionsSchema.optional()])) - .output(z.promise(z.record(z.string(), z.unknown()))) + .output(z.promise(z.record(z.string(), z.unknown()).nullable())) .describe('Update record'), /**