Uh oh!
There was an error while loading. Please reload this page.
feat(objectql): engine option bags are a closed contract — unknown keys throw instead of silently doing nothing - #4400
Merged
Conversation
…re spellings The @example taught filters/sort/top — keys a driver can never receive: the engine folds/rejects wire spellings before this layer (#4346, #4371), and QuerySchema declares where/orderBy/limit. Also dedupes the doubled @returns tag. Comment-only; no generated artifact reads @example (all 8 gates verified up to date). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gs are a closed contract — unknown keys throw (#4371 option 2) All six engine methods now reject non-null option keys they do not execute, naming the legal set per method; cursor/distinct quote their #4286 tombstone; null stays a withdrawal. Legal sets = schema keys + documented extras (searchFields — now DECLARED on EngineQueryOptionsSchema, it was read and sent all along; onFieldsDropped on update; driver pass-through keys on the methods whose bag reaches driver options — count/aggregate forward nothing, so they reject those too). A drift pin holds the sets equal to the schemas. Same sweep also closes what the option-2 caller survey turned up: - bag object no longer overrides the resolved AST object ({...query, object} order fix); protocol refuses a contradicting POST-body object (400 QUERY_OBJECT_MISMATCH) and strips its own vocabulary (object/count/ tombstones/non-aggregate having) off the engine bag - nested expand ASTs reject the four wire-only spellings like the top level - $search/$searchFields engine reads removed (protocol normalizes to bare) - DbQueueAdapter purge/purgeFailed passed {id} — purge deleted NOTHING (throw swallowed into warn), purgeFailed always threw; both now pass {where: {id}}, and the test fake stops accepting the signature the real engine rejects - 19 test bags carrying the dead 'filters' key cleaned up (two were intent-carrying: a locale multi-update and an oidc dogfood assert that both silently scanned unfiltered) - driver.find JSDoc example shows canonical QueryAST keys Closes#4371 (option 1 landed in #4387). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 4 package(s): 110 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang added a commit
that referenced
this pull request
Aug 1, 2026
…, and executes every option it declares (#4419) (#4459) findOne reads a single row, which makes its predicate the only thing between the caller and an arbitrary record. When the predicate is missing the result is not `null` — it is the object's FIRST ROW: a real, plausible-looking record with nothing to do with the request, which the `if (!row)` check every call site already has cannot catch, and which then propagates into whatever is computed next. #4419 reported this against the `filter` key, which #4346 (fold on every entry point) and #4400 (unknown keys throw) already closed. This is what those left standing. BREAKING: findOne refuses a query that selects nothing in particular. findOne(o) / findOne(o, {}) / findOne(o, { where: {} }) -> findOne(o, { where: … }) the record matching this -> findOne(o, { search: 'Acme' }) the record this search finds -> findOne(o, { orderBy: [{ field, order }] }) the FIRST in this order -> find(o, { limit: 1 }) any row will do, said aloud The error names all four. `find` and `count` are unchanged — returning or counting every row is an honest answer. The guard reads the CALLER's predicate, before RLS/sharing middleware injects its own. Two silent drops that produced the same wrong record are fixed with it: - findOne({ search }) now applies the search. The ADR-0061 expansion lived in find() alone while both methods are checked against the SAME legal-key set, so `search` passed the gate, reached a driver that does not read it, and the read ran unpredicated. - MongoDBDriver.findOne now applies orderBy, fields and offset. It translated `where` and dropped the rest, so "the newest record" returned whichever document the scan reached first. No ordering is imposed when the caller supplies none (#4363) — unchanged on both drivers. And a drift pin walks ENGINE_OPTION_KEY_SETS.findOne requiring each declared key to have an observable effect, so the next declared-but-unexecuted key fails CI instead of shipping. The Mongo cases live in one shared table read by both a server-free options suite and the real-mongod suite, so neither half can drift.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes#4371 — option (2), the complete answer (option (1) landed in #4387). Scoped by a full caller survey of every engine call site in this repo +
cloud, plus every key the engine itself reads.What changes
All six engine methods reject non-null option keys they do not execute, naming the legal set. Retired keys (
cursor/distinct) quote their #4286 tombstone.nullstays a withdrawal, same as the fold's rule.Per-method legal keys = the method's
Engine*OptionsSchemakeys plus the documented extras the survey proved live:searchFields— read by the engine's$searchexpansion and sent by the protocol layer all along, but never declared onEngineQueryOptionsSchema; now declared (spec) and legal (engine).onFieldsDroppedonupdate— contract-declared write observability (service-automation: update_record reportssuccesswhen written fields are silently stripped — no observability for dropped writes (split from #3356) #3407), unrepresentable in the serializable schema.transaction/tenantId/tenantIds/timezone/bypassTenantAudit/preserveAudit) onfind/findOne/update/delete— the bag IS the driver-options base there (pinned behaviour).count/aggregateforward nothing, so they reject these instead of accepting-and-ignoring.A drift pin asserts each set against its schema's
.shape(minus tombstones, plus the extras), so a key added to the spec cannot be silently rejected.What the survey turned up and this PR also closes
objectoverrode the resolved object on the AST —{ object, ...query }spread order letquery.objectwin, splittingast.objectfrom the table actually queried (drivers use the first argument; middleware and hooks readingast.objectwere lied to). AST now keeps the resolved name; a direct call passingobjectis rejected; the protocol refuses a POST-bodyobjectcontradicting the route (400QUERY_OBJECT_MISMATCH) instead of picking a winner.findDataleaked protocol vocabulary onto the engine bag (object,count,joins,windowFunctions,cursor,distinct, non-aggregatehaving) — now stripped at the boundary; wire behaviour unchanged.expand: { rel: { sort } }) reject the four wire-only spellings exactly like the top-level bag did in fix(objectql): reject wire-only alias spellings (sort/select/skip/populate) on direct engine calls #4387.$search/$searchFieldsengine reads removed — the protocol normalizes to bare keys; a direct call passing the OData spelling now throws instead of half-working onfindonly.DbQueueAdapter.purge/purgeFailedwere broken on the real engine: both passed{ id }, a key the engine never read —purgedeleted nothing (every delete threw "Delete requires an ID" into a warn-level catch) andpurgeFailedalways threw. The test fake'sdeleteimplemented the{ id }signature the real engine rejects, which is exactly how this stayed green. Both fixed to{ where: { id } }; the fake now matches the real contract.filterskey cleaned up — two were intent-carrying (a locale multi-update and an oidc dogfood assert that both silently scanned unfiltered).driver.findJSDoc example showed wire spellings a driver can never receive — now canonical.Tests
engine-unknown-option.test.ts(per-method rejection, tombstones, null withdrawal, extras legal where they work and rejected where they don't,objectrejection, nested-expand rejection + canonical expand works, drift pin).protocol-data.test.ts(protocol strips its keys; matching bodyobjecttolerated; contradicting bodyobject400s before the engine).Wire/HTTP callers are unaffected. Direct engine callers passing junk keys get an error that names the fix.
🤖 Generated with Claude Code