fix(sharing,runtime): a sort passed straight to the engine never ordered anything; migrate in-repo call sites to canonical keys (#4346) - #4370
Merged
Conversation
…dered anything (#4346) Sweeping every in-repo engine call site that still spoke a deprecated alias turned up three that were not cosmetic. #4346 made the engine fold `filter`→`where` and `top`→`limit` on all six methods. The remaining four pairs in RPC_QUERY_ALIAS_SLOTS (select, sort, skip, populate) fold at the RPC/wire layer only — their values need shape lowering that belongs to those layers — and a DIRECT engine.find() never crosses that layer. Three call sites passed `sort` there, so it rode onto the AST untouched, every driver's `Array.isArray(query.orderBy)` guard declined to emit an ORDER BY, and the read returned an ordinary-looking, arbitrarily ordered result: share-link-routes.ts shared AI conversation messages, created_at asc runtime/domains/share-links.ts the same route, runtime-domain copy share-link-service.ts listLinks: "the 200 most recent" share links Each pairs the dropped sort with a limit — the "latest N" shape whose failure #4226 spelled out, one layer below the normalizer #4226 fixed. listLinks had no test at all, which is why it went unnoticed; it is pinned now on the option bag the engine RECEIVES, not on row order, because the failure is that the key never becomes `orderBy` and a double honouring either spelling passes either way. Verified the pin fails against the pre-fix line before keeping it. The other 27 sites are strict no-ops since #4346 folds `filter`: approvals 5, auth 2, reports 6, sharing 11, webhooks 2, plus a spec doc example teaching `filters` (a wire-only alias the engine does not fold at all, so the example taught a call that matches every row). Renaming them stops the framework depending on a spelling it asks users to migrate off. Service-level `filter` PARAMETERS — each service's own public API — are deliberately untouched. One test double had to move with them: approver-org-scope's fake engine read `opts.filter`, so it kept passing only because both sides shared the deprecated dialect. That is the mechanism that let the whole class persist. 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 7 package(s): 115 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Contributor
Author
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…e now throws instead of silently dropping it (#4371) (#4387) 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: 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.
#4346 的收尾清扫。原计划是"把仓内
{filter}调用迁到where"的机械 PR,但扫描扫出了三个活 bug,所以这个 PR 有两种性质的改动。一、活 bug:直连引擎的
sort从未排过序#4346 让引擎在六个方法上折叠
filter→where和top→limit。RPC_QUERY_ALIAS_SLOTS里另外四对(select/sort/skip/populate)只在 RPC/wire 层折叠——它们的值需要形状降级(sort的 record 形式、populate的名字列表),那属于那一层的职责——而直接调engine.find()根本不经过那一层。三处调用在那里传了
sort,于是它原样骑在 AST 上,每个 driver 的Array.isArray(query.orderBy)守卫都拒绝生成 ORDER BY:share-link-routes.tscreated_at ascruntime/domains/share-links.tsshare-link-service.tslistLinks三处都是"丢掉的排序 +
limit"——正是 #4226 注释点名的 "latest N" 失败模式:未生效的排序返回任意顺序,limit再把它切成任意一页,而响应看起来完全正常。#4226 修的是 wire 归一化器,这三处在它下面一层。如何确证的(不靠读代码):写了运行时探针,用真实引擎 + 记录型 driver 打印引擎实际收到的 AST ——
针脚:
listLinks此前没有任何测试,这是它坏了这么久没人发现的原因。新针脚断言引擎收到的选项包而非行顺序——因为失败点是"这个键从未变成orderBy",而一个两种拼写都认的测试替身无论如何都会通过。保留前先把修复临时回退,确认针脚会变红(expected undefined to deeply equal [...]),不是恒绿装饰。二、清理:27 处空操作重命名
其余 27 处引擎调用的
filter改为where(approvals 5、auth 2、reports 6、sharing 11、webhooks 2,外加 spec 一处文档示例)。因 #4346 已折叠,这些是严格的空操作——意义在于框架不再依赖它要求用户迁离的拼写,这是将来退役别名的前置条件。system-names.ts示例教的是engine.find({ filters: [...] })——filters是 wire-only 别名,引擎层根本不折,所以那个示例教的调用匹配全表。与 [P1]filterfolds towhereinengine.findonly —findOne/count/update/deletesilently match EVERY row, and the hook docs teach the broken call #4346 修掉的 hook 示例同一类。filter参数(如listRequests(filter)、listLinks(filter))刻意不动——那不是引擎选项包。扫描器从调用出发遍历(而非 grepfilter:),正是为了区分这两者:仓内filter:有数千处命中,绝大多数是无关的。三、一处测试替身必须同步
approver-org-scope的假引擎读opts.filter,所以它一直通过仅仅因为替身和生产代码讲同一种废弃方言。这正是让整类问题得以长期存活的机制,值得单独记一笔。已改为读规范键。顺带全仓扫了其他替身:多数读opts.where ?? opts.filter(有韧性),无其他需要同步的。验证
plugin-sharing 226 ✅ / plugin-approvals 330 ✅ / plugin-auth 579 ✅ / plugin-reports 50 ✅ / plugin-webhooks 25 ✅ / runtime 974 ✅ / spec 7160 ✅ / objectql 1450 ✅;改动包 typecheck ✅;
check:docs✅(生成物无漂移)。遗留
另外四对别名在引擎层依然静默失效——一个未来的调用者写
select/skip/populate会踩同样的坑,而选项类型是any,编译期抓不到。这个更深的缺口(引擎应当对无法理解的查询键报错而非静默忽略)会按 PD #10 单独开 issue,不在本 PR 扩大范围。Refs #4346, #4226, #3795.
🤖 Generated with Claude Code