fix(engine): ObjectRepository declares the findOne / update shapes it already published

.changeset/objectql-scoped-repository-declared-returns.md

@@ -0,0 +1,21 @@
+---
+"@objectstack/objectql": minor
+---
+
+feat(engine): `ObjectRepository.findOne` / `.update` publish their honest types — the contract's shapes, not `any` (#16786)
+
+**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<any>` 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<Record<string, any> | null>`
+- `update` → `Promise<Record<string, any> | number | null>`
+
+`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<any>`, 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<any>`, so the class already matches its contract and there is no drift to repair on this side. That half stays open on #16786.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves. No metadata key, no spec key, no authored property, no config field, no accepted request shape and no stored artifact changes spelling or shape; `packages/spec` is untouched, so `objectstack migrate meta` has nothing to rewrite, `spec-changes.json` has nothing to project and the upgrade guide has no row to gain. What moves is the declared RETURN TYPE of two TypeScript methods, and the rewrite this ships -- narrow the `null` arm -- is addressed to a TYPESCRIPT CONSUMER and delivered by the compiler at their own call site, which is the audience the ADR-0087 ledger explicitly does not serve.
+     `type-surface-only` is the category built for exactly this class and it is NOT claimed here, because its predicate 4 cannot be made to name this change's symbols -- measured, not assumed. The bare form `engine.ts#findOne` resolves to the FIRST same-named member in the file, `ObjectQL.findOne` (line 9761), which #16783 already narrowed, so predicate 4 reads `narrowed-from-erased is FALSE: at the merge base ... was already CONCRETE` -- a true statement about a member this diff never touched. The documented fallback, a dotted member path, is walked only through OBJECT-LITERAL nesting and refuses a class member: `ObjectRepository.findOne does not resolve: no ObjectRepository object literal is declared`. Both narrowed members are class members whose names repeat in the file, so neither spelling can address them. The gap is reported on the card rather than worked around, and the **BREAKING** banner above is carried rather than dropped -- which is the erosion #13080 was filed about. -->

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<string, any> | 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<any>` 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 () => {

packages/objectql/src/engine.ts

@@ -14707,7 +14707,23 @@ export class ObjectRepository implements IScopedObjectRepository {
     });
   }
 
-  async findOne(query: any = {}): Promise<any> {
+  /**
+   * [#16786] Declared `Promise<Record<string, any> | null>`, not `Promise<any>`.
+   *
+   * `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<Record<string, any> | 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<any> {
+  /**
+   * [#16786] Declared `Promise<Record<string, any> | 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<any>` 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<Record<string, any> | number | null> {
     return this.engine.update(this.objectName, data, {
       ...options,
       context: this.context,

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<Record<string, any> | null>` and `update` as
+// `Promise<Record<string, any> | 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<any>`.
+//
+// `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<Record<string, string>>): Map<string, ts.Diagnostic[]> {
+  const dir = resolve(PKG, 'src/__scoped_repo_probes__');
+  const paths = new Map<string, string>();
+  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<string, ts.Diagnostic[]>();
+  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<code>: <message>`, 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<T> = 0 extends (1 & T) ? true : false;
+type Row = Awaited<ReturnType<ReturnType<ScopedContext['object']>['findOne']>>;
+export const isAny: IsAny<Row> = 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<string, any> | null'],
+    ['public-door-findOne', 'Record<string, any> | null'],
+    ['sudo-door-findOne', 'Record<string, any> | 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<string, any> | 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<Row>` is `false` once the declaration is honest, so assigning
+    // `true` to it is an error. When `Row` is `any`, `IsAny<Row>` 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'");
+  });
+});