Skip to content

[P2] A direct engine call silently drops sort/select/skip/populate — declared query contract, zero enforcement #4371

Description

@os-zhuang

Found while doing the #4346 follow-up sweep of in-repo engine call sites (#4370).

#4346 made ObjectQL fold filterwhere and toplimit on all six methods. The other four pairs in RPC_QUERY_ALIAS_SLOTSselectfields, sortorderBy, skipoffset, populateexpand — fold at the RPC/wire layer only, because their values need shape lowering (sort's {field: 'asc'} record form, populate's name list → the expand record) that legitimately belongs to those layers.

That is a defensible split. What is not defensible is the failure mode when a direct engine.find() — which never crosses that layer — is handed one of the four:

// packages/objectql/src/engine.ts (find)
const ast: QueryAST = { object, ...query };   // `sort` rides along verbatim

The key lands on the AST untouched, drivers read only the canonical name, and the request succeeds with the parameter silently discarded. Measured against a real engine + recording driver:

>>> AST keys: object,sort,limit
>>> ast.orderBy = undefined
>>> ast.sort    = [{"field":"created_at","order":"asc"}]

This is not hypothetical — it already shipped three times

#4370 fixes three call sites that hit exactly this, all pairing the dropped sort with a limit (the "latest N" shape #4226 called out):

call site asked for got
plugin-sharing/src/share-link-routes.ts shared AI conversation messages, created_at asc arbitrary order
runtime/src/domains/share-links.ts the same route, runtime-domain copy arbitrary order
plugin-sharing/src/share-link-service.ts listLinks the 200 most recent share links an arbitrary 200

Nothing caught them: EngineQueryOptions does not declare sort, but every engine facade types its options bag any (and the sites carried as any), so TS said nothing; the engine ignored the key; the drivers ignored the key; the responses were ordinary.

This is the #4226 diagnosis one layer down. #4226's own words on why an unapplied sort is the worst kind of silent failure — "the rows are all there and all real — which is exactly why nobody notices" — apply verbatim here, except #4226 fixed the normalizer and these calls sit below it.

The gap in one line

The engine declares a query contract (EngineQueryOptionsSchema) and then enforces none of it: an undeclared key is neither folded, nor rejected, nor logged. That is the declared ≠ enforced shape of AGENTS.md PD #10, and the remedy the repo has used before (#1475, #3106) is to make the runtime answer for what the contract states.

Suggested fix

Make an engine option bag that carries a key the engine cannot honour fail loudly instead of silently dropping it. Roughly, in the shared entry-point helper #4346 already added (foldEngineOptionAliases):

  1. Reject the four wire-only aliases at the engine boundary with a message naming the canonical key and the layer that folds it — e.g. sort → "sort is a wire spelling folded by the protocol layer; a direct engine call must pass orderBy (SortNode[])." Cheap, exact, and it converts every future instance of this bug into an immediate throw. Note the shape lowering is the reason NOT to fold them here; rejecting costs nothing and keeps the layering.
  2. Consider rejecting genuinely unknown keys too (anything outside QUERY_AST_KEYS + the engine's own option names). This is the stronger version and matches what metadata-protocol already does at the wire boundary (UNSUPPORTED_QUERY_PARAM, app-showcase 权限模型复测清单(07-15 对照 main 重评):1 项回归级重大缺陷(#2930 锚点写护栏被 A4 词表打穿)+ 2 项部分修复 + 3 项未修 + 1 项建议降级 + 2 项观察 #2926/REST 列表:未知查询参数被静默当作字段过滤器 —— ?pageSize=5 返回 200 + 空列表 #4134) — the engine is the one ingress with no such gate. Needs a survey of internal callers first; several pass extra bookkeeping keys today.
  3. Pin each: {sort} alone on a direct engine call is refused; {orderBy} works; the RPC/wire path still folds sort as it does now (the fold must NOT move).

(1) alone closes the reachable hole and is small. (2) is the complete answer and deserves its own scoping.

Why not "just fold them at the engine too"

Tempting, but it makes the engine a second place that lowers sort's record spelling and populate's list spelling into canonical shapes — the exact per-reader re-implementation #3795 existed to delete, and the reason #4346 deliberately scoped the engine fold to the two slots that need no lowering. Rejecting keeps one fold, one owner.

Refs #4346, #4370, #4226, #3795, #3713, ADR-0049, AGENTS.md PD #10 / #12.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions