Skip to content

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

Draft
claude[bot] wants to merge 3 commits into
mainfrom
claude/issue-16786-scoped-object-repository-return
Draft

fix(engine): ObjectRepository declares the findOne / update shapes it already published#17255
claude[bot] wants to merge 3 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

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 patch, and why this is auditable

Nothing is widened and no symbol is added. packages/spec/src/contracts/scoped-context.ts
already publishes the narrower type; this is an implementation coming back to
the declaration it had already published
, not a contract that moved. That is
the patch rung.

Checked against the recorded WHICH LEVEL maintainer ruling of 2026-09-04
(decision batch #35, on #15294, recorded in .github/workflows/pr-automation.yml):
it puts additive widening of a published surface — a new exported symbol, a new
accepted key or value — at minor, and leaves "changes no public surface" at
patch. This PR adds no exported symbol and widens no accept set.

For contrast, the sibling narrowing PR #16783 shipped minor with a BREAKING
banner, and its own changeset gives the reason: "this PR also widens
@objectstack/objectql's index with new exported symbols, which that ruling puts
at minor on its own"
— three new registered ADR-0112 error codes. This PR adds
none, so that driver is absent here.

The type-axis consequence is stated plainly in the changeset rather than hidden by
the grade: a consumer that typed against the concrete class (rather than the
contract, which already said this) and reads a field off findOne's result
without a null check now gets the compiler asking for that check.

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

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 6058cb229a6a65f0cbec1084dd68f865b9bec269packageMentionDocs.

Which tree this was computed on

This run read content/docs from 289e2da0651f870cdbb8556e19618cc37b14e1db — the merge of head ac5c48dbad1cb5529237fb6dcc40579688ef8617 into base 6058cb229a6a65f0cbec1084dd68f865b9bec269, 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 289e2da0651f870cdbb8556e19618cc37b14e1db && git checkout 289e2da0651f870cdbb8556e19618cc37b14e1db
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6058cb229a6a65f0cbec1084dd68f865b9bec269 ac5c48dbad1cb5529237fb6dcc40579688ef8617 && git checkout -B drift-repro 6058cb229a6a65f0cbec1084dd68f865b9bec269 && git merge --no-ff ac5c48dbad1cb5529237fb6dcc40579688ef8617

node scripts/docs-audit/affected-docs.mjs --json 6058cb229a6a65f0cbec1084dd68f865b9bec269

⚠️ 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 6058cb229a6a65f0cbec1084dd68f865b9bec269 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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.

1 participant