fix(objectql): reject wire-only alias spellings (sort/select/skip/populate) on direct engine calls - #4387
Merged
Conversation
…e now throws instead of silently dropping it (#4371) The engine folds filter→where and top→limit itself (#4346); the other four pairs in RPC_QUERY_ALIAS_SLOTS fold at the RPC/protocol layer only, because their value shapes need lowering that belongs there. A direct engine.find() never crosses that layer, so the alias key rode the AST verbatim, drivers read only the canonical name, and the request succeeded with the parameter silently discarded — three shipped instances fixed in #4370, and a fourth in the engine's own seedAutonumber (select — its projection never applied, and the surrounding catch would have swallowed the new rejection into 'seed from 0', i.e. duplicate autonumbers; now passes fields). find/findOne now reject a non-null wire-only spelling with an error naming the canonical key, the layer that owns the fold, and the target shape. Folding them here instead would re-implement the RPC layer's shape lowering per reader — the #3795 condition — so rejecting keeps one fold, one owner. The where-only methods (update/delete/count/aggregate) are deliberately unchanged: their contracts honour no sort/projection/pagination in either spelling, so 'pass orderBy instead' would redirect to a key they silently ignore too — that is #4371 option (2), scoped separately. Call-site survey (this repo + cloud): zero direct callers pass these keys today; every wire ingress folds at the protocol layer (pinned end-to-end). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 13 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…ys throw instead of silently doing nothing (#4400) * docs(spec): driver.find example shows canonical QueryAST keys, not wire 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> * feat(objectql,spec,metadata-protocol,service-queue): engine option bags 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> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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 free
to 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.
Implements option (1) of #4371 — the small, exact fix that closes the reachable hole. Refs #4371 (kept open for option (2): unknown-key enforcement, which needs its own caller survey).
The bug
#4346 made the engine fold
filter→whereandtop→limiton all six methods. The other four pairs inRPC_QUERY_ALIAS_SLOTS—select→fields,sort→orderBy,skip→offset,populate→expand— fold at the RPC/protocol layer only, because their value shapes need lowering (sort's{field: 'asc'}record form,populate's name list) that belongs there. A directengine.find()never crosses that layer: the key rode the AST verbatim, drivers read only the canonical name, and the request succeeded with the parameter silently discarded —sort+limit("the latest N") returning an arbitrary N. Three shipped instances were fixed in #4370.The fix
find/findOnenow reject a non-null wire-only spelling:ENGINE_WIRE_ONLY_SLOTSis derived as the complement of the slots the engine folds — the spec table stays the ONE place the mapping is declared ([P2]protocol.tsimplements 4 of the 5 documented RPC alias precedences backwards — and disagrees withhttp-dispatcher.tson three of them #3795 discipline), and a seventh pair added there lands on exactly one side of the split.protocol.tsimplements 4 of the 5 documented RPC alias precedences backwards — and disagrees withhttp-dispatcher.tson three of them #3795 condition — and [P1]filterfolds towhereinengine.findonly —findOne/count/update/deletesilently match EVERY row, and the hook docs teach the broken call #4346 deliberately scoped the engine fold to the two slots needing no lowering. One fold, one owner; the wire path is pinned unchanged end-to-end (real protocol → real engine).nullstays a withdrawal (rides through for drivers to ignore), matching the fold's own rule.update/delete/count/aggregate) deliberately do NOT get this rejection: their contracts honour no sort/projection/pagination in either spelling, so "passorderByinstead" would redirect the caller to a key those methods silently ignore too. That is option (2)'s territory.The fourth in-repo instance, found in the engine's own house
seedAutonumberpassedselect: ['id', field]— the projection never applied (the scan worked only because an unprojected row still carries the field). Worse, itscatch { return 0 }would have swallowed the new rejection into "seed from 0", i.e. duplicate autonumbers. Now passesfields.Call-site survey (why this is safe to throw)
Balanced-paren scan of every
.find(/.findOne(argument text across this repo and thecloudrepo, plus the indirect channels (ObjectRepository/ScopedContext, sandbox body facade, plugin-auth adapter, settings/CLI engine wrappers, metadata database-loader, flowget_recordsexecutor): zero remaining callers pass these keys to a direct engine call. Every wire ingress (REST, RPC, http-dispatcher) folds at the protocol layer. Driver-contract tests assertingdriver.findignores non-QueryAST keys are a different layer and untouched.Survey leftovers filed/noted: #4386 (pre-existing
executeDataActionquery-fallback bug found while surveying); expand-nested alias keys (expand: {rel: {sort}}) are still silently dropped — noted on #4371 for option (2)'s scoping.Tests
12 new pins in
engine-wire-alias-reject.test.ts: each of the four spellings refused onfind(driver untouched),findOne({sort})refused, SortNode[]-shapedsortstill refused (the KEY is the contract), multi-alias single report, canonical bag reaches the driver,nullwithdrawal,{filter, top}fold unregressed, and wiresort/select/skipthrough the real protocol layer folding to canonical and passing the guard.Local: objectql 1462, runtime 995, rest 539, cli 629, client 204, metadata-protocol 136, plugin-sharing 226, plugin-approvals 330, plugin-dev 24, plugin-pinyin-search 14, verify 7 — all green;
objectqltypecheck clean.🤖 Generated with Claude Code