Skip to content

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

Merged
os-sam merged 4 commits into
mainfrom
claude/issue-16786-scoped-object-repository-return
Sep 10, 2026
Merged

fix(engine): ObjectRepository declares the findOne / update shapes it already published#17255
os-sam merged 4 commits into
mainfrom
claude/issue-16786-scoped-object-repository-return

Conversation

@claude

@claude claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Part of #16786

Clause-②: no

(Declared by the claiming seat, not by the implementer — the declaration is the seat's judgement (check-clause2-carriers.mjs: ⛔ do not fill the line in on the claiming seat's behalf). This PR puts no new key on a published payload, adds no export, no closed-set member and no registry entry; it narrows two members that were already declared narrow on IScopedObjectRepository. The C5 widening-tell limb therefore has nothing to collide with. The Check Changeset LEVEL AXIS refused for a MISSING declaration, not for a wrong grade — carrier: needs:contract-review is not on this PR (0 label(s) read) and declaration line: the PR body carries no Clause-②: line.)

Deliberately not Fixes. The card carries two gaps and this PR lands one of
them; the seat ruling on the card keeps the other — IScopedObjectRepository.updateById,
which lives in packages/spec and belongs to the domain:spec seat — as an
unfixed remainder, so this PR must not close the card on merge. The half left
open is named under "What this deliberately does not do" below.

Type shapes below are written with square brackets (Promise[…]) rather
than angle brackets: GitHub's body sanitizer eats short tag-shaped fragments,
backticked ones included.

The defect

ObjectRepository.findOne and .update declared Promise[any].

Both sit between two declarations that are already narrow:

The class received a narrow value and re-widened it back to any on the way out.
implements does not catch that: a wider declared return always satisfies a
narrower one, so class ObjectRepository implements IScopedObjectRepository
compiled green the entire time while the members it published were any.

So the interface's narrowing reached only call sites whose static type is the
interface. 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 I measured, and where it refines the card

Probes compiled against packages/objectql/src on origin/main ae19f5edb7,
before any edit:

probe reading
ctx: HookContext ; ctx.api!.object(n).findOne(…) already narrow — TS2322 naming the record-or-null shape
api: ScopedContext ; api.object(n).findOne(…) any — 0 diagnostics
ql.createContext({}).object(n).findOne(…) any — 0 diagnostics
IsAny[Awaited[ReturnType[…findOne]]] on the class door resolves true — confirms any

⚠️ The first row refines the card's framing. The card says the hook-facing
call ctx.api.object(name).findOne(…) "does not resolve through the narrowed
interface". For a handler typed (ctx: HookContext) => … that is not the
case: HookContext.api was narrowed to IScopedContext by #5945 / #6311, so
that exact spelling reads the narrow type today and read it before this PR too.

The defect the card names is real; the door it named is not the one that was
wide. The any lives on the class-typed doors above, which is where the fix
and the pin both go. A probe written through HookContext is green on both sides
of this change and would have pinned nothing.

Why minor + BREAKING — re-graded, and why the first grade was wrong

This PR was first graded patch, reasoning that packages/spec/src/contracts/scoped-context.ts
already publishes the narrower type, so the class coming into line moves no
contract. That reasoning is wrong, and the landed precedent refutes it in almost
the same words.

PR #15280 (merged 2026-09-04, 2200f8ec89) narrowed SqlDriver.update() and the
TursoDriver.update() override off an explicit Promise[any]. I read its diff and
both changesets. Its files are driver src + tests + two changesets — no new
exported symbol, and packages/spec untouched
. Both changesets are minor, each
opening "BREAKING for TypeScript consumers — a published TYPE-surface
narrowing, shipped as minor under the launch-window convention"
. Its stated
reason is my own patch argument, verbatim:

IDataDriver.update() declares Promise[Record[string, unknown] | null], and an
explicit any satisfies that structurally — so the emitted .d.ts read
Promise[any] and no caller holding a SqlDriver … was ever asked to narrow.

That is this PR exactly: contract already narrow, implementation re-widening,
.d.ts publishing any. #15280 also names PR #14434 doing the same on
@objectstack/driver-memory, so it is a convention rather than one PR's choice.

My earlier reading leaned on #16783's minor being driven by its new exported
symbols (three registered ADR-0112 error codes) and inferred that a pure narrowing
might stay patch. #15280 isolates that variable and refutes the inference: no new
symbols, still minor + BREAKING.

The break is real here for the same reason. ObjectRepository is exported from
packages/objectql/src/index.ts:128, and the narrowed declarations reach the
published .d.ts (evidence below), so a consumer typed against the concrete class
— rather than against the contract, which already said this — now gets the compiler
asking for the null check.

⚠️ Two sibling PRs of the same shape shipping different grades would itself be a
defect, and sibling card #15267 is landing this same family. This PR now matches.

ADR-0087 disposition: no-migration-prescription, the category sibling #16783
used for the same family. type-surface-only is semantically the right category and
is deliberately NOT claimed, because its predicate 4 cannot address either narrowed
symbol — measured in both spellings, and recorded in the marker itself:

  • bare packages/objectql/src/engine.ts#findOne resolves to the FIRST same-named
    member in the file, ObjectQL.findOne (line 9761), which feat(engine)!: findOne, update and delete declare what they answer, and their hook seams are guarded (#16231) #16783 already narrowed,
    so predicate 4 correctly reports narrowed-from-erased is FALSE … already CONCRETE — a true sentence about a member this diff never touches;
  • 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. Reported as a finding rather than worked around, and the
BREAKING banner is carried rather than dropped — which is the erosion #13080 was
filed about.

Census

The in-repo census for this change was one file: engine-filter-alias.test.ts,
reading .status off a value that can be null. Repaired here with the file's
own expect(...).not.toBeNull() / ! idiom, which also makes the assertion
non-vacuous — under Promise[any] that test's two spellings agreed trivially if
both lookups returned null.

The test-typecheck debt ledger is unchanged at 44 files / 242 errors: nothing
was added to it.

The pin, and its ablation

packages/objectql/src/scoped-repository-return-narrowing.test.ts drives
ts.createProgram over probe files — the idiom
packages/spec/src/contracts/scoped-context.test.ts already uses. Every negative
probe asserts 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.

Reverse verification, run from the committed state, mutating src/engine.ts back
to Promise[any] on both members:

BEFORE mutation  narrow findOne decl : 1   narrow update decl : 1
AFTER  mutation  narrow findOne decl : 0   wide findOne decl  : 1   wide update decl : 1
HEAD blob 7b04e12856b35de2383592b298364b877954aa78
mutated   3dbc70d17165a5750406308a899b69baa513d2be     (proves the mutation reached disk)

RED LEG    Tests  5 failed | 1 passed (6)
           failure signature: AssertionError: expected '' to contain 'TS2322'
           -- i.e. the probes report NO diagnostic, which IS the `any` reading
           the 1 pass is the anti-vacuity control, which must stay green

RESTORE    git checkout HEAD -- PATH
           blob back to 7b04e12856b35de2383592b298364b877954aa78
           git diff HEAD empty, git status clean
GREEN LEG  Tests  6 passed (6)

No rebuild is involved in either leg: the probes resolve ../engine to
src/engine.ts directly, so the mutation is visible to the compiler without a
dist/ round trip.

Verification

check result
pnpm lint (full repo, eslint . --no-inline-config) exit 0, no findings
pnpm --filter @objectstack/objectql typecheck exit 0 (src + tsconfig.scripts.json + test-layer ratchet)
pnpm --filter @objectstack/objectql test exit 0 — 289 files / 4861 tests passed
derived gate families (scripts/pm/dispatch-gates.mjs --commands) 61 of 63 exit 0
pnpm check:dual-build-cjs-loads NOT MEASUREDPREREQUISITE NOT MET, exit 3; needs a full-repo pnpm build. Its own words: "This is NOT a pass: nothing was measured." CI owns that build.
pnpm check:type-check-debt NOT MEASURED — same shape, exit 3, needs the built closure. Its own words: "NOT a pass and NOT a finding".

Published-surface evidence for the changeset (built, then grepped in the path
files[] actually ships):

packages/objectql/dist/util-CRhA5389.d.ts:7447
    findOne(query?: any): Promise[Record[string, any] | null];
packages/objectql/dist/util-CRhA5389.d.ts:7462
    update(data: any, options?: any): Promise[Record[string, any] | number | null];

What this deliberately does not do

  • IScopedObjectRepository.updateById is untouched. It is still a
    Promise[any] in packages/spec, and that surface belongs to the domain:spec
    seat. It stays open on [finding] Ruling A on #16231 narrows IScopedObjectRepository, but ctx.api.object(name) resolves through the CLASS ObjectRepository — the hook-facing door keeps Promise<any>, and updateById keeps it too #16786 as the unfixed remainder — which is why this PR
    says Part of, not a closing keyword. Note the class side needs nothing here:
    ObjectRepository.updateById already matches what the contract declares, so
    there is no drift to repair on this side.
  • ScopedContext.object's return annotation is left as ObjectRepository.
    The card's suggested shape (explicitly "not asserted") was to annotate it as
    IScopedObjectRepository. I measured that route and did not take it: the class
    is deliberately wider than the contract in MEMBERSHIP — create, delete,
    deleteById, aggregate, execute — and the class's own docblock records that
    as intentional. Annotating the accessor removes those members from every
    class-typed call site. Measured census for that route: 2 files / 8 errors,
    six of them member-removal breaks (Property 'delete' does not exist…,
    Property 'execute' does not exist…) in engine-filter-alias.test.ts and
    engine-repo-execute-elevation.test.ts. Repairing those would mean either
    casting around the fix or deleting live coverage. Narrowing the two drifting
    members instead fixes strictly more doors (the class door, the exported
    engine door, and a directly-held ObjectRepository), removes nothing, and
    leaves the census at 1 file / 2 errors — both of which are the narrowing
    working, not breakage.
  • Region fence respected. PR fix(objectql)!: beforeUpdate receives the persist image; the caller submission moves to ctx.submitted (#16344) #17195 is open against packages/objectql/src/engine.ts
    in lines 11298–11801. This diff's hunk headers are @@ -14710 and @@ -14728
    roughly 2,900 lines clear of the fenced region.
  • No file under packages/spec/ is touched. No governed surface is touched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU


Generated by Claude Code

…lready publishes

`IScopedObjectRepository.findOne` / `.update` declare `Record<string, any> | null`
and `Record<string, any> | 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<any>` 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
…ared repository

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 10, 2026
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

13 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/data-flow.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/automation/hook-bodies.mdx (via ObjectRepository (symbol, a top-level class), findOne (symbol, a method of class ObjectRepository))
  • content/docs/automation/webhooks.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/kernel/contracts/data-engine.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/kernel/contracts/index.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/kernel/events.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/permissions/attachments-access.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/permissions/field-level-security.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/permissions/record-view-auditing.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/permissions/rls.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/permissions/system-context.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/protocol/objectql/schema.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/ui/react-pages.mdx (via findOne (symbol, a method of class ObjectRepository))

5 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/releases/v16.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/releases/v17/17-0.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/releases/v17/17-4.mdx (via findOne (symbol, a method of class ObjectRepository))
  • content/docs/releases/v17/index.mdx (via findOne (symbol, a method of class ObjectRepository))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json cf6e0a193b665e512c1e05a499464aae689ea838packageMentionDocs.

Which tree this was computed on

This run read content/docs from 460a9e569d4cca0db4718157b000f99eb4b8a6de — the merge of head 6b1ab96140472c5cc425a170d8c831459e9b1a3f into base cf6e0a193b665e512c1e05a499464aae689ea838, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 460a9e569d4cca0db4718157b000f99eb4b8a6de && git checkout 460a9e569d4cca0db4718157b000f99eb4b8a6de
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cf6e0a193b665e512c1e05a499464aae689ea838 6b1ab96140472c5cc425a170d8c831459e9b1a3f && git checkout -B drift-repro cf6e0a193b665e512c1e05a499464aae689ea838 && git merge --no-ff 6b1ab96140472c5cc425a170d8c831459e9b1a3f

node scripts/docs-audit/affected-docs.mjs --json cf6e0a193b665e512c1e05a499464aae689ea838

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs cf6e0a193b665e512c1e05a499464aae689ea838 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… BREAKING

Landed precedent PR #15280 measured: `SqlDriver.update()` and the `TursoDriver.update()`
override moved off an explicit `Promise<any>` 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants