Skip to content

fix(metadata-protocol): refuse a sort naming a formula field instead of dropping it silently (#6994) - #7096

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-6994-formula-orderby-gate
Aug 9, 2026
Merged

fix(metadata-protocol): refuse a sort naming a formula field instead of dropping it silently (#6994)#7096
os-zhuang merged 2 commits into
mainfrom
claude/issue-6994-formula-orderby-gate

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #6994

结论先说:定位在 ingress,但只关上了一半门

排序轴上 assertSortFieldsExist 已经会拒绝不存在的字段(#4226)和带点的路径(#4256),
但对一个「确实存在、不带点、然而类型上根本不落列」的字段没有任何判定 —— formula 字段就在
对象的字段表里,所以它过了 unknown 检查;它不带点,所以它过了 dotted 检查;然后它抵达一个
没有这一列的 driver,排序被静默丢弃。

这次在本分支 base(2f3e79351)上用真实 SqlDriver(better-sqlite3,落盘)+ 真实
ObjectQL + 本协议
重新测过,五行按 C A E B D 插入,sort_keyrecord.title
formula:

repro_contact physical columns: ["id","created_at","updated_at","title","seq","account_id","child_total"]
formula col `sort_key` present: false

CONTROL   orderBy title asc            ["A","B","C","D","E"]     真实列确实能排
BASELINE  no sort                      ["C","A","E","B","D"]     插入顺序

FORMULA   orderBy sort_key asc         ["C","A","E","B","D"]     5 rows, 200
   (its sort_key values)               ["C","A","E","B","D"]
FORMULA   orderBy sort_key desc        ["C","A","E","B","D"]
   asc === desc (byte-identical)?      true

RAW SQL   order by sort_key            sqlite: no such column: sort_key
PROTOCOL  {"sort":"sort_key"}          200, 5 records, ["C","A","E","B","D"]

ascdesc 逐字节相同,这才说明是排序被丢掉而不是巧合。机制与 issue 描述一致:
SqlDriver.createColumnformula 直接 return(虚拟字段,读时计算),sqlite 报
no such column,#3821 的未知列兜底重试时去掉了 ORDER BY。响应里甚至原样带着调用方
要求排序的那一列的值,乱序摆着,还报 200 —— 请求与应答当面矛盾,却自称成功。

为什么修在 ingress,而不是 driver 或查询编译器

派单把「路由归属」当作未决问题带了进来,所以先给证据再给结论。

同一扇门上的两个邻居已经就是这么做的。 这不是给 engine 侧问题打的补丁,而是补上这个 gate
自己四条轴里缺的第三种判定:

unknown 形状 类型
search unknown unsearchable(已存在,消息里直接写 type '${meta.type}')
expand unknown notRelations(已存在,"only lookup / master_detail / tree fields can be expanded")
sort unknown dotted 本 PR 之前:没有

「字段存在,但这条轴用不了它的类型」在 protocol.ts 里早就是 ingress 的职责,已落地、已测试。
sort 是这一族里唯一没长出这条判定的成员,formula 正是从这个缺口漏过去的。

不选 driver(方案 2)的理由是可测的。 #3821 兜底对真正未知的列是刻意且正确的
(注释写明它保护的是无 registry 的 cloud 多租户运行时),把它改成拒绝就破坏了它存在的理由;
而 driver 手上只有一条 SQL 错误,分不清「formula,契约上就不落列」和「这台 host 没有 registry
所以列查不到」—— 这个知识在 schema 里,不在错误消息里。更关键的是 driver-memory 根本不报错
(行上没有这个 key,比较全相等 → 稳定序),所以 driver 定位必须在 sql / memory / turso /
mongodb / sqlite-wasm 各写一遍,没有共享接缝:一个契约 N 份实现,正是 contract-first 的反面。

不选 materialize(方案 3)的理由。 与代码库多处声明的「formula 是虚拟的」契约冲突。还有一个
更隐蔽的陷阱:engine 其实可以在 applyFormulaPlan 之后事后排序,看起来像修好了,但 driver
早已分页,事后排的只是那一页任意顺序的行 —— 小结果集上像能用,一加 limit/offset 就错。

没有走 STOP 的边界在哪。 剩下的一半确实在 engine:engine.find() 直连的内部调用方
(hook / flow / report / expand 子查询)不经过本 gate。但把那一半关上,要先决定
engine.find拒绝还是保持它有文档记载的内部调用方宽容 —— 那是 engine-core 的契约决策,
不是 gate 修复。已按 #7095 单独立项(含三个选项、各自代价,以及一个「engine 事后排序」的陷阱
说明),没有在本 PR 里替它做主。

ingress-only 留下的洞:实测,不是推测

同一个脚本在打了补丁之后再跑一次,两半同时可见:

PROTOCOL  {"sort":"sort_key"}          REFUSED 400 INVALID_SORT: … a formula field on 'repro_contact' …
PROTOCOL  {"orderBy":["sort_key"]}     REFUSED 400 INVALID_SORT
PROTOCOL  {"sort":"-sort_key"}         REFUSED 400 INVALID_SORT

FORMULA   orderBy sort_key asc         ["C","A","E","B","D"]     engine.find 直连:依旧静默
   asc === desc (byte-identical)?      true

所以这条不能当成「已关闭」来读。测试里对它有一条明确标注为
RECORD OF A KNOWN HOLE 的 pin:它记录的是未修好的行为,并写明 engine-core 关掉它的那天
这条测试应该变红,到时候一起更新 —— 而不是被后来的人当成「直连路径没问题」的证据。

顺带在 objectui 侧留了 objectstack-ai/objectui#3950:grid 目前只对引用型字段关掉排序表头
(isExpandableFieldType),formula 列仍可点击排序 —— 本 PR 之后那一次点击会从「什么也没发生」
变成一个 400。两者错因相同:UI 提供了平台做不到的排序。

summary / rollup 不在这一族

summary 拿到的是真实且被维护的 table.float 列,排序真的有效(#6924 已测:
orderBy 一个 summary 字段 desc → E D C B A,值为 5 4 3 2 1),autonumber 同理是真实字符串列。
所以拒绝集是「不落列」而不是「computed」:spec 里的 COMPUTED_VALUE_TYPES
(formula/summary/autonumber)是写入契约(never client-written),拿它来卡排序会
误伤两个本来能用的类型。测试里为此留了一条会因过度拒绝而变红的对照。

判定优先级

unknown > dotted > 不落列 —— 与 expand gate 的 unknown > not-a-reference 同序:
先身份,再形状,最后类型。前两条判定逐条未变,并各有一条 pin 固定。

消息措辞

#6924(dotted)和 #6673(search 轴)逐字一致:
「Denormalise the value onto '...' (a stored field, written when the source changes) and sort
by that.」有一条测试专门 pin 这个一致性本身,任一扇门单独改词就会变红 —— #4256#6673
当初正是这样走散的。

改了哪些文件

文件 改动
packages/metadata-protocol/src/protocol.ts 新增模块级 UNMATERIALIZED_SORT_TYPES(今天就是 {formula},附三种 computed 类型的落列对照表与五处既有拼写的账);assertSortFieldsExist 的 dotted 分支由 early-return 改为块,其后新增第三条判定;assertSortFieldsExist 的文档补 #6994 段与 SCOPE 段(原文里「NOT dotted, so this gate lets it through」在本 PR 后已不成立,一并修正)
packages/objectql/src/query-expression-conformance.test.ts fixture 增 sort_key(formula)与 subtask_total(summary,取值刻意让 asc / desc / 插入序 / 标题序两两不同,避免对照空转);新增 8 种 wire 拼写的拒绝用例 + 消息契约 + 两扇门措辞一致性 + 两条优先级 pin + 1 条 known-hole pin + 1 条 summary 对照;stub driver 的 find 改为拷贝返回行
.changeset/sort-formula-field-refusal.md minor,含实测转录与 scope 说明

其中 stub driver 那一处值得单说:它原本把自己 store 里的行引用直接交出去,而
applyFormulaPlan 会把算出来的 formula 值写回它拿到的那条记录 —— 于是读一次就把虚拟值持久化
进了「数据库」
,第二次读真的按一个没有任何 driver 拥有的列排了序。真实 SqlDriver
asc / desc 是逐字节相同的,这个 double 与它所替身的 driver 结论相反。这是我在反向验证里
预测失败发现的,不是顺手美化。

验证

  • pnpm --filter @objectstack/objectql test162 files / 2800 tests passed
  • pnpm --filter @objectstack/metadata-protocol test65 files / 827 tests passed
  • pnpm --filter @objectstack/objectql typecheck → clean
  • eslint --no-inline-config(两个改动文件)→ clean
  • check:nul-bytes / check:error-code-casing / check:route-envelope / check:empty-changeset /
    check:doc-authoring / check:adr-anchors → OK
  • check:query-options-erasure —— 本地先红后绿:pin 里两处 as any 让 test surface 从
    256 涨到 258。这里的 options 并非故意越界({ orderBy: [{ field, order }] } 本就是合法的
    EngineQueryOptionsParsed),所以按 gate 给的第一条路补类型,而不是用它为越界输入准备的
    as unknown as EngineQueryOptions 出口。现已回到 256,none new。

反向验证(方向先预测,后测量)

预测 结果
R1 移除本 gate → 8 条拼写用例 + 2 条消息用例变红 ✅ 10 failed / 95 passed,与预测数目一致
R1 两条优先级 pin 在两个方向都绿 ✅ 两向皆绿 —— 它们是护栏,不是本 PR 的证据
R1 summary 对照在两个方向都绿 ✅ 两向皆绿 —— 只会因过度拒绝变红
R1 known-hole pin 在两个方向都绿 ✅ 两向皆绿 —— 它 pin 的正是没修的那一半
R2 只撤销 stub driver 的行拷贝 → 仅 known-hole pin 变红 ✅ 1 failed / 104 passed,其余无影响

一处预测失败,连同原因一并留档: known-hole pin 第一次写成「asc 与 desc 相同」时变红
(desc 回来 E D C B A)。原因不是产品代码,而是上面那条 stub driver 的引用泄漏 —— 真实 driver
的实测(asc === desc)反过来证明了 double 是错的那一方,于是修 double 而不是改断言迁就它。

没有留下未测的预测。

corpus 计数(按 widening 纪律,先数再改)

例子仓里共 6 个 formula 字段(showcase budget_remaining / f_formula,CRM is_closed /
expected_revenue / days_to_close / full_name),没有任何一处按它们排序;例子应用里
真正声明的排序目标是 budget(currency)、estimate_hours(number)、due_date(date),
都是存储类型。所以这条拒绝在 corpus 上零误伤,它只拒绝本来就已经坏掉的调用。


Generated by Claude Code

claude added 2 commits August 9, 2026 14:37
`assertSortFieldsExist` refused an unknown field (#4226) and a dotted path
(#4256) but not a real, non-dotted field whose TYPE materialises no column.
A `formula` field is in the object's field map, so it passed the unknown
check, and carries no dot, so it passed the dotted check — then reached a
driver with no column for it, where the #3821 backstop retried the query
without the ORDER BY and answered 200 in arbitrary order.

Adds the axis' third verdict, the one `assertSearchFieldsExist`
(`unsearchable`) and `assertExpandFieldsExist` (`notRelations`) already
have: known field, wrong type for this axis. Precedence stays
`unknown` > `dotted` > unmaterializable, so both older verdicts are
unchanged. `summary`/`autonumber` are deliberately NOT in the set — they
get real columns and sort correctly.

Also copies rows out of the conformance stub driver: it handed back live
references into its own store, so `applyFormulaPlan` persisted computed
formula values back into "the database" and a second read really sorted by
a column no driver has — the double contradicting the driver it stands in
for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw
`check:query-options-erasure` counts `as any` on engine query options and
the ratchet went red (test surface 256 -> 258). The options here are NOT
off-contract — `{ orderBy: [{ field, order }] }` is a legitimate
`EngineQueryOptionsParsed` — so the remedy is typing them, not the
`as unknown as EngineQueryOptions` escape the gate offers for deliberately
bad input. Ratchet back to 256, none new.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 9, 2026 2:53pm

Request Review

@github-actions github-actions Bot added the size/m label Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/metadata-protocol)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Copy link
Copy Markdown
Contributor Author

ACCEPT — PM step-7 review (domain:metadata seat, session session_01W6bLax4KMrSfnE1ydFU8Dw). Marking ready + auto-merge (SQUASH).

CI on af7e34d7d: 25 check runs, 0 failures — 23 completed: success including ESLint (the job carrying the family gates) and TypeScript Type Check, plus Build Core, Test Core 1-3/3, Dogfood Regression Gate 1-3/3, Dogfood Verify CLI, Temporal Conformance, Check Changeset, Check PR Size; 2 skipped by path filter. Changeset present (minor).

The routing question this card carried is settled, and by structural evidence rather than preference. I dispatched it with the routing live — I had suggested domain:engine-core when filing it, triage said domain:metadata, and I told the dev to STOP-and-report if the locus turned out to be the driver. The answer is better than either of us had: the same gate already answers "known field, wrong TYPE for this axis" on two of its four axes — assertSearchFieldsExist splits unknown from unsearchable, assertExpandFieldsExist splits unknown from notRelations. SORT had only unknown and dotted. So this is not an engine-side fix pushed through the metadata lane; it is the one member of an existing ingress family that never grew its third verdict, which is exactly the gap formula fell through. Triage's routing was right for the half that was actionable, and my instinct was right about a different half — see below.

The question I made mandatory was answered honestly, and the answer is "no, it does not close it". An ingress-only fix covers everything reaching findData (REST list, POST /data/:object/query, the export route's $orderby, the RPC dispatcher) but not internal callers reaching engine.find() directly, which still drop the sort silently. Rather than let that ambiguity live in a PR body, the dev put a pin in the suite explicitly labelled RECORD OF A KNOWN HOLE that asserts the unfixed behaviour and says it should go red the day the other half lands. That is the right shape: the gap is now a test that will notice when it closes, not a sentence someone has to remember. Filed as #7095 for domain:engine-core with three costed options, because closing it means deciding whether engine.find refuses or keeps its documented internal-caller tolerance — a contract decision, not a gate fix.

Three more things worth putting on the record:

  • A trap avoided by measurement. The spec's COMPUTED_VALUE_TYPES is {formula, summary, autonumber} and looks like the predicate to reuse — but it is the write contract. Gating a sort with it would refuse two types that sort correctly. The measured family is exactly {formula}: SqlDriver.createColumn has one early return, driver-turso skips the same one, summarytable.float, autonumbertable.string.
  • Zero false positives, counted before changing per the widening discipline: 6 formula fields across the example corpus, none a sort target; the sorts actually declared are all stored types. The refusal only rejects calls that were already broken.
  • check:query-options-erasure went red locally and was fixed the hard way — two as any had pushed the surface 256 → 258, and the remedy was to type the options rather than take the as unknown as EngineQueryOptions escape the gate offers. Back to 256.

The missed prediction is the most useful paragraph in the report. The known-hole pin went red on first write, and the cause was not product code: the conformance stub driver handed back live references into its own store, so applyFormulaPlan writing computed values onto those records persisted the virtual value — and the next read genuinely sorted by a column no driver has. The real-SqlDriver measurement (asc === desc, byte-identical) is what settled which side was wrong: the double was contradicting the driver it stands in for. The double was fixed and the assertion was not weakened, then R2 was run specifically to check whether that row-copy was load-bearing elsewhere — it was not. A test double that is more capable than the real thing is a defect generator, and catching it this way is exactly what reverse verification is for.

Also noted and appreciated: the self-criticism about taking the fix out with git checkout <base> -- <path> before committing and overwriting uncommitted work. The patch-file / temp-commit recipe exists for that, and R2 was run after committing.

Out-of-lane finding objectui#3950 (ObjectGrid disables the sort header only for reference types, so a formula column stays click-sortable — after this PR that click turns from "nothing happens" into a 400) is correctly filed rather than absorbed, with the unverified part about RelatedList stated as unverified instead of asserted.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 9, 2026 17:05
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 9, 2026
Merged via the queue into main with commit 9f7a7c2 Aug 9, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-6994-formula-orderby-gate branch August 9, 2026 17:16
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.

A non-dotted orderBy naming a formula field answers 200 in arbitrary order — the sort is silently dropped (measured on driver-sql + driver-memory)

2 participants