From bd0afc6655e69aa9256ade69959eb27b4e8edeea Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 00:13:15 +0000 Subject: [PATCH 1/4] fix(engine): ObjectRepository declares the findOne/update shapes it already publishes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `IScopedObjectRepository.findOne` / `.update` declare `Record | null` and `Record | number | null`, and `IDataEngine` — the call each of these forwards to — declares the same. `ObjectRepository` sat between two narrow declarations and re-widened the value back to `Promise` on the way out, which `implements IScopedObjectRepository` accepts (a wider return always satisfies a narrower one) while every call site reaching a repository through the CLASS kept reading `any`, `ObjectQL.createContext(…).object(n).findOne(…)` included. Census: one consumer, `engine-filter-alias.test.ts`, which read `.status` off a value that can be null. Repaired with the file's own `not.toBeNull()` / `!` idiom. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU --- .../objectql/src/engine-filter-alias.test.ts | 11 +++++-- packages/objectql/src/engine.ts | 31 +++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/packages/objectql/src/engine-filter-alias.test.ts b/packages/objectql/src/engine-filter-alias.test.ts index 4d1846d9c4..3e7e02a81a 100644 --- a/packages/objectql/src/engine-filter-alias.test.ts +++ b/packages/objectql/src/engine-filter-alias.test.ts @@ -217,8 +217,15 @@ describe('filter → where folds on every engine method (#4346)', () => { const repo = ctx.object('task'); const viaWhere = await repo.findOne({ where: { status: 'done' } }); const viaFilter = await repo.findOne({ filter: { status: 'done' } }); - expect(viaFilter.status).toBe('done'); - expect(viaWhere.status).toBe('done'); + // [#16786] `repo.findOne` declares `Record | null`, so the + // null both spellings could return is asserted away rather than read + // through — the same `expect(row).not.toBeNull()` / `row!` idiom this + // file already uses above. Under the old `Promise` this pair + // agreed vacuously if BOTH lookups came back null. + expect(viaFilter).not.toBeNull(); + expect(viaWhere).not.toBeNull(); + expect(viaFilter!.status).toBe('done'); + expect(viaWhere!.status).toBe('done'); }); it('a cleanup hook calling repo.delete({filter, multi}) no longer empties the object', async () => { diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 16f986a16f..7b04e12856 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -14707,7 +14707,23 @@ export class ObjectRepository implements IScopedObjectRepository { }); } - async findOne(query: any = {}): Promise { + /** + * [#16786] Declared `Promise | null>`, not `Promise`. + * + * `IScopedObjectRepository.findOne` has declared that shape since #16231's + * ruling A landed (PR #16783), and `IDataEngine.findOne` — the call this + * method forwards to, one line down — declares it too. This method sat + * between two narrow declarations and re-widened the value back to `any` on + * the way out, so `implements IScopedObjectRepository` stayed satisfied (a + * wider return always satisfies a narrower one) while every call site that + * reaches a repository through the CLASS rather than the interface kept + * reading `any` — `ObjectQL.createContext(…).object(n).findOne(…)` among + * them, which is exported. + * + * ⛔ Not a narrowing of the contract: the contract already said this. This + * is the implementation coming back to the declaration it published. + */ + async findOne(query: any = {}): Promise | null> { return this.engine.findOne(this.objectName, { ...query, context: this.context, @@ -14725,7 +14741,18 @@ export class ObjectRepository implements IScopedObjectRepository { return this.insert(data); } - async update(data: any, options: any = {}): Promise { + /** + * [#16786] Declared `Promise | number | null>`, the same + * re-widening as {@link findOne} and repaired the same way: the record for + * the single-record form, the affected-row count for the predicate form + * (`{ where, multi: true }`), `null` when the write matched nothing. + * + * ⛔ `updateById` is deliberately NOT touched here. Its `Promise` is + * what `IScopedObjectRepository.updateById` itself declares, so the class + * matches its contract and there is no drift to repair on this side; that + * member is `packages/spec`'s to narrow and stays open on #16786. + */ + async update(data: any, options: any = {}): Promise | number | null> { return this.engine.update(this.objectName, data, { ...options, context: this.context, From 9f3d43e7dfe746f64fd71feb96bd28f7d05be057 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 00:14:59 +0000 Subject: [PATCH 2/4] test(engine): pin that a class-typed `object(name)` hands back a declared repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiler-driven probes (`ts.createProgram`, the idiom `packages/spec/src/contracts/scoped-context.test.ts` uses) over the exported class doors — `ScopedContext`, `ObjectQL.createContext`, `sudo()` — asserting the diagnostic NAMES the declared shape, so neither a bare "it errored" nor an `any` that erased the type can satisfy it. Anti-vacuity: the legal spelling must compile clean and no probe may report TS2307. Probes go through the CLASS, not `HookContext`: `HookContext.api` was narrowed to `IScopedContext` by #5945, so a `(ctx: HookContext)` probe is green on both sides of this fix and pins nothing. Measured, and recorded in the file header. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU --- ...scoped-repository-return-narrowing.test.ts | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 packages/objectql/src/scoped-repository-return-narrowing.test.ts diff --git a/packages/objectql/src/scoped-repository-return-narrowing.test.ts b/packages/objectql/src/scoped-repository-return-narrowing.test.ts new file mode 100644 index 0000000000..88d269d9b7 --- /dev/null +++ b/packages/objectql/src/scoped-repository-return-narrowing.test.ts @@ -0,0 +1,192 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import ts from 'typescript'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// ─── [#16786] the repository a CLASS-typed call site reaches is declared ──── +// +// `IScopedObjectRepository` (`packages/spec/src/contracts/scoped-context.ts`) +// declares `findOne` as `Promise | null>` and `update` as +// `Promise | number | null>` — ruling A on #16231, landed +// as PR #16783. `IDataEngine`, the call each `ObjectRepository` member forwards +// to, declares the same shapes. `ObjectRepository` sat between those two narrow +// declarations and re-widened the result back to `Promise`. +// +// `implements` does not catch that: a WIDER declared return always satisfies a +// narrower one, so `class ObjectRepository implements IScopedObjectRepository` +// compiled green while the members it published were `any`. The interface's +// narrowing therefore reached only the call sites whose STATIC type is the +// interface — and the doors this package exports are typed as the CLASS: +// +// ObjectQL.createContext(ctx).object(n) -> ScopedContext -> ObjectRepository +// ScopedContext.sudo().object(n) -> ObjectRepository +// engine.transaction((trxCtx) => …) -> ScopedContext -> ObjectRepository +// +// ## What this file measures, and what it deliberately does not +// +// Measured on `origin/main` ae19f5edb7 before the fix, with these probes: +// +// ctx: HookContext ; ctx.api!.object(n).findOne(…) -> ALREADY NARROW +// api: ScopedContext ; api.object(n).findOne(…) -> `any` +// ql.createContext({}).object(n).findOne(…) -> `any` +// +// ⚠️ The first line is why the probes below are written through the CLASS and +// the exported engine door rather than through `HookContext`. `HookContext.api` +// was narrowed to `IScopedContext` by #5945/#6311, so a handler typed +// `(ctx: HookContext) => …` reads the narrow type today and read it before this +// fix too — a probe written that way is GREEN on both sides and pins nothing. +// The `any` lives on the class-typed doors, so that is where the probes go. +// +// ## Why the compiler API rather than `@ts-expect-error` +// +// The same reason `packages/spec/src/contracts/scoped-context.test.ts` gives: +// `@ts-expect-error` is satisfied by ANY error on the next line, and this +// file's whole subject is WHICH type a call resolves to. Every negative probe +// below asserts that the diagnostic NAMES the declared shape, so a bare "it +// errored" — or an `any` that erased the type entirely — cannot satisfy it. +// +// Anti-vacuity: a harness that resolves nothing reports zero diagnostics and +// looks exactly like success, so `control-legal` must compile CLEAN, and no +// probe may report TS2307 (unresolved module). + +const HERE = dirname(fileURLToPath(import.meta.url)); +const PKG = resolve(HERE, '..'); + +/** + * Compile probe files against this package's real `src/engine.ts` and return + * each one's diagnostics. The probes live (virtually) beside the source, so + * `../engine` resolves the way any sibling module would and `@objectstack/spec` + * resolves the way a real consumer's does — through the installed package. + */ +function compileProbes(probes: Readonly>): Map { + const dir = resolve(PKG, 'src/__scoped_repo_probes__'); + const paths = new Map(); + for (const [name, text] of Object.entries(probes)) paths.set(resolve(dir, `${name}.ts`), text); + + const options: ts.CompilerOptions = { + target: ts.ScriptTarget.ES2020, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + strict: true, + skipLibCheck: true, + noEmit: true, + // A probe declares a const and stops; TS6133 is an opinion about the + // probe's framing, not about whether the call is well-typed. + noUnusedLocals: false, + noUnusedParameters: false, + types: ['node'], + baseUrl: PKG, + }; + + const host = ts.createCompilerHost(options, true); + const realGetSourceFile = host.getSourceFile.bind(host); + const realFileExists = host.fileExists.bind(host); + const realReadFile = host.readFile.bind(host); + host.getSourceFile = (fileName, languageVersion, onError, shouldCreate) => { + const overlay = paths.get(resolve(fileName)); + return overlay === undefined + ? realGetSourceFile(fileName, languageVersion, onError, shouldCreate) + : ts.createSourceFile(fileName, overlay, languageVersion, true); + }; + host.fileExists = (fileName) => paths.has(resolve(fileName)) || realFileExists(fileName); + host.readFile = (fileName) => paths.get(resolve(fileName)) ?? realReadFile(fileName); + + const program = ts.createProgram([...paths.keys()], options, host); + const out = new Map(); + for (const name of Object.keys(probes)) out.set(name, []); + for (const d of ts.getPreEmitDiagnostics(program)) { + const file = d.file?.fileName ? resolve(d.file.fileName) : undefined; + for (const name of Object.keys(probes)) { + if (file === resolve(dir, `${name}.ts`)) out.get(name)!.push(d); + } + } + return out; +} + +/** One diagnostic per line, `TS: `, for readable assertions. */ +function render(diagnostics: readonly ts.Diagnostic[]): string { + return diagnostics + .map((d) => `TS${d.code}: ${ts.flattenDiagnosticMessageText(d.messageText, ' ')}`) + .join('\n'); +} + +const PROBES = { + // ── anti-vacuity: the harness really resolves and really compiles ──────── + 'control-legal': ` +import type { ObjectQL } from '../engine'; +export const probe = async (ql: ObjectQL) => { + const row = await ql.createContext({}).object('task').findOne({ where: { id: 't1' } }); + return row === null ? 'missing' : String(row.status); +};`, + // ── the class-typed door: what a hook reaches at RUNTIME ───────────────── + 'class-door-findOne': ` +import type { ScopedContext } from '../engine'; +export const probe = async (api: ScopedContext) => { + const bad: number = await api.object('task').findOne({ where: { id: 't1' } }); +};`, + // ── the exported public door ───────────────────────────────────────────── + 'public-door-findOne': ` +import type { ObjectQL } from '../engine'; +export const probe = async (ql: ObjectQL) => { + const bad: number = await ql.createContext({}).object('task').findOne({ where: { id: 't1' } }); +};`, + // ── the elevated door ──────────────────────────────────────────────────── + 'sudo-door-findOne': ` +import type { ScopedContext } from '../engine'; +export const probe = async (api: ScopedContext) => { + const bad: number = await api.sudo().object('task').findOne({ where: { id: 't1' } }); +};`, + // ── update carries the same repair ─────────────────────────────────────── + 'class-door-update': ` +import type { ScopedContext } from '../engine'; +export const probe = async (api: ScopedContext) => { + const bad: boolean = await api.object('task').update({ id: 't1', status: 'done' }); +};`, + // ── the direct any-detector, in case a future edit reaches `any` by ────── + // ── some route the assignment probes above do not cover ───────────────── + 'not-any-findOne': ` +import type { ScopedContext } from '../engine'; +type IsAny = 0 extends (1 & T) ? true : false; +type Row = Awaited['findOne']>>; +export const isAny: IsAny = true;`, +} as const; + +describe('[#16786] `object(name)` hands back a DECLARED repository, not `any`', () => { + const diagnostics = compileProbes(PROBES); + + it('resolves every probe against real source (anti-vacuity)', () => { + for (const [name, ds] of diagnostics) { + expect(render(ds), `${name} failed to resolve its imports`).not.toContain('TS2307'); + } + // The legal spelling — null handled — must compile with nothing to say. + // Without this, a harness that compiled nothing would satisfy every + // negative probe below by reporting no diagnostics at all. + expect(render(diagnostics.get('control-legal')!)).toBe(''); + }); + + it.each([ + ['class-door-findOne', 'Record | null'], + ['public-door-findOne', 'Record | null'], + ['sudo-door-findOne', 'Record | null'], + // TypeScript normalises this union's order; the string is the compiler's + // own rendering, not the source order in `IScopedObjectRepository`. + ['class-door-update', 'number | Record | null'], + ])('%s: the diagnostic NAMES the declared shape', (probe, declared) => { + const text = render(diagnostics.get(probe)!); + // Not merely "some error": `any` produces NO error here, and an erased or + // widened declaration produces one that does not name this shape. + expect(text).toContain('TS2322'); + expect(text).toContain(declared); + }); + + it('findOne is not `any` — measured by the compiler, not by reading the source', () => { + // `IsAny` is `false` once the declaration is honest, so assigning + // `true` to it is an error. When `Row` is `any`, `IsAny` is `true` + // and this probe compiles clean — which is the pre-fix reading. + const text = render(diagnostics.get('not-any-findOne')!); + expect(text).toContain('TS2322'); + expect(text).toContain("Type 'true' is not assignable to type 'false'"); + }); +}); From ac5c48dbad1cb5529237fb6dcc40579688ef8617 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 00:38:10 +0000 Subject: [PATCH 3/4] chore(engine): changeset for the declared repository return shapes Graded `patch`: nothing is widened and no symbol is added. The contract already published these shapes; the implementation is coming back to a declaration it had already published. Checked against the recorded WHICH LEVEL ruling of 2026-09-04 (decision batch #35, on #15294), whose `minor` trigger is additive widening. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU --- ...ctql-scoped-repository-declared-returns.md | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 .changeset/objectql-scoped-repository-declared-returns.md diff --git a/.changeset/objectql-scoped-repository-declared-returns.md b/.changeset/objectql-scoped-repository-declared-returns.md new file mode 100644 index 0000000000..186b897f20 --- /dev/null +++ b/.changeset/objectql-scoped-repository-declared-returns.md @@ -0,0 +1,39 @@ +--- +"@objectstack/objectql": patch +--- + +fix(engine): `ObjectRepository` declares the `findOne` / `update` shapes it already published (#16786) + +`ObjectRepository.findOne` and `.update` declared `Promise` and now declare +what `IScopedObjectRepository` — the contract this class carries an `implements` +clause for — has declared since #16231's ruling A landed (PR #16783): + +- `findOne` → `Promise | null>` +- `update` → `Promise | number | null>` + +**Why this is a `patch` and not a `minor`.** Nothing is widened and no symbol is +added. `packages/spec/src/contracts/scoped-context.ts` already publishes the +narrower type, and `IDataEngine` — the call each of these two methods forwards to, +one line down — already publishes it too. This class sat between two narrow +declarations and re-widened the result back to `any` on the way out. `implements` +does not catch that, because a WIDER declared return always satisfies a narrower +one: `class ObjectRepository implements IScopedObjectRepository` compiled green +the whole time while the members it published were `any`. So this is an +implementation coming back to the declaration it had already published — the +repo's `patch` rung — and not a contract that moved. The recorded **WHICH LEVEL** +maintainer ruling of 2026-09-04 (decision batch #35, on #15294, recorded at +`.github/workflows/pr-automation.yml`) puts *additive widening* of a published +surface — a new exported symbol, a new accepted key or value — at `minor`; this +PR does none of those, and adds no exported symbol. + +**Who has to change something, on the TYPE axis.** A TypeScript consumer that +typed against the concrete `ObjectRepository` / `ScopedContext` class — rather +than the `IScopedObjectRepository` contract, which already said this — and reads +a field off `findOne`'s result without a null check, or off `update`'s result +without separating the by-id record from the predicate-form count. Those call +sites were reading `any`; they now read the declared shape and the compiler asks +for the null check. Consumers already written against the contract, including +every hook whose `ctx` is typed `HookContext` (`HookContext.api` has been +`IScopedContext` since #5945), see no change: they were already narrow. + +The in-repo census for this change was one file, repaired here. From 6b1ab96140472c5cc425a170d8c831459e9b1a3f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 02:12:57 +0000 Subject: [PATCH 4/4] chore(engine): re-grade the repository return declarations to minor + BREAKING Landed precedent PR #15280 measured: `SqlDriver.update()` and the `TursoDriver.update()` override moved off an explicit `Promise` onto the shape `IDataDriver` already declared -- no new exported symbol, `packages/spec` untouched -- and both changesets shipped `minor` with a **BREAKING** banner. That is this change's shape exactly, so the earlier `patch` reasoning ("the contract already published it, so nothing moved") is the very fact pattern that precedent grades `minor`: the emitted `.d.ts` read `any`, so no caller holding the class was ever asked to narrow. The ADR-0087 disposition is `no-migration-prescription`, as sibling PR #16783 used for the same family. `type-surface-only` is semantically the right category but its predicate 4 cannot address either narrowed symbol; the marker records that measurement. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU --- ...ctql-scoped-repository-declared-returns.md | 44 ++++++------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/.changeset/objectql-scoped-repository-declared-returns.md b/.changeset/objectql-scoped-repository-declared-returns.md index 186b897f20..0cd2a59e90 100644 --- a/.changeset/objectql-scoped-repository-declared-returns.md +++ b/.changeset/objectql-scoped-repository-declared-returns.md @@ -1,39 +1,21 @@ --- -"@objectstack/objectql": patch +"@objectstack/objectql": minor --- -fix(engine): `ObjectRepository` declares the `findOne` / `update` shapes it already published (#16786) +feat(engine): `ObjectRepository.findOne` / `.update` publish their honest types — the contract's shapes, not `any` (#16786) -`ObjectRepository.findOne` and `.update` declared `Promise` and now declare -what `IScopedObjectRepository` — the contract this class carries an `implements` -clause for — has declared since #16231's ruling A landed (PR #16783): +**BREAKING** for TypeScript consumers — a published TYPE-surface narrowing, shipped as `minor` under the launch-window convention (the one PR #15280 used for `SqlDriver.update()` and the `TursoDriver.update()` override, and PR #14434 before it on `@objectstack/driver-memory`). + +`ObjectRepository.findOne()` and `.update()` were written out with an explicit `Promise` while they have always answered what the contract declares — each one forwards, one line down, to an `IDataEngine` door that already declares the shape: - `findOne` → `Promise | null>` - `update` → `Promise | number | null>` -**Why this is a `patch` and not a `minor`.** Nothing is widened and no symbol is -added. `packages/spec/src/contracts/scoped-context.ts` already publishes the -narrower type, and `IDataEngine` — the call each of these two methods forwards to, -one line down — already publishes it too. This class sat between two narrow -declarations and re-widened the result back to `any` on the way out. `implements` -does not catch that, because a WIDER declared return always satisfies a narrower -one: `class ObjectRepository implements IScopedObjectRepository` compiled green -the whole time while the members it published were `any`. So this is an -implementation coming back to the declaration it had already published — the -repo's `patch` rung — and not a contract that moved. The recorded **WHICH LEVEL** -maintainer ruling of 2026-09-04 (decision batch #35, on #15294, recorded at -`.github/workflows/pr-automation.yml`) puts *additive widening* of a published -surface — a new exported symbol, a new accepted key or value — at `minor`; this -PR does none of those, and adds no exported symbol. - -**Who has to change something, on the TYPE axis.** A TypeScript consumer that -typed against the concrete `ObjectRepository` / `ScopedContext` class — rather -than the `IScopedObjectRepository` contract, which already said this — and reads -a field off `findOne`'s result without a null check, or off `update`'s result -without separating the by-id record from the predicate-form count. Those call -sites were reading `any`; they now read the declared shape and the compiler asks -for the null check. Consumers already written against the contract, including -every hook whose `ctx` is typed `HookContext` (`HookContext.api` has been -`IScopedContext` since #5945), see no change: they were already narrow. - -The in-repo census for this change was one file, repaired here. +`IScopedObjectRepository` — the contract this class carries an `implements` clause for — declares both, and has since ruling A on #16231 landed (PR #16783). An explicit `any` satisfies that structurally, because a **wider** declared return always satisfies a narrower one: `class ObjectRepository implements IScopedObjectRepository` compiled green the whole time while the emitted `.d.ts` read `Promise`, so no caller holding an `ObjectRepository` — or reaching one through `ScopedContext` or `ObjectQL.createContext()`, both exported from this package's index — was ever asked to narrow. They are now declared as the contract declares them. No runtime behaviour changes. + +A caller that read fields off `findOne()`'s result through the `any` now narrows the `null` arm first; a caller that read `update()`'s result now separates the by-id record from the predicate-form count. The in-repo census for this change was one file, repaired alongside. + +`updateById` is deliberately untouched: `IScopedObjectRepository.updateById` itself declares `Promise`, so the class already matches its contract and there is no drift to repair on this side. That half stays open on #16786. + +