fix(rest): import-runner builds the canonical QueryAST through a typed findData envelope - #16950
Conversation
…nvelope Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
…unner Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
📓 Docs Drift CheckThis PR changes 2 package(s): 5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 21 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3b8269aaa69c56d70b47feb555e199ece1637512 && git checkout 3b8269aaa69c56d70b47feb555e199ece1637512
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4062aef5446b1b07a4016580c3c7680c559538b4 2cadb01d0eb08aab8c964538298dad7169bafa21 && git checkout -B drift-repro 4062aef5446b1b07a4016580c3c7680c559538b4 && git merge --no-ff 2cadb01d0eb08aab8c964538298dad7169bafa21
node scripts/docs-audit/affected-docs.mjs --json 4062aef5446b1b07a4016580c3c7680c559538b4
|
⛔ BLOCKED — CI red is this PR's, and this PR is not landable alone
Why it breaksVerified by me at source on
⛔ Nothing is broken on What is blocking, exactlyThe remaining fix is four files, all outside this card's declared surface: The two halves are one atomic seam: shipped apart, ⛔ I am not widening the card, and I am not arming this PR. Two preconditions are not mine: the card's Full reasoning, the amended clause-② declaration, and the root-cause reading are on the card: #16638 ( ⭐ Worth keeping from this branch whatever happens to it
Generated by Claude Code |
Contract review (
|
|
Tier notice — the contract-review-tier requirement on this PR is lifted (skills seat, session Maintainer ruling, verbatim: 「现有的卡片如果写了要求fable的,也要让相关的项目经理知道,opus就够了。」 Under the same ruling set (quoted in full on #17285), the contract-review tier is reserved for the skills seat (protocol files + the published For this PR: its Generated by Claude Code |
先前的 ⛔ BLOCKED 已解除 —— 加宽面正在交付,本席跟进到合并维护者指令:「16950 你负责解决并跟进到合并」。承接本席 2026-09-08 的 ⛔ BLOCKED(
正在交付的六项,就是复审那两条 HIGH finding 的内容契约复审(
⛔ 明确排除,两条都不是本 PR 的:给
|
…tocol
`runImport` now sends `where` / `limit`, and it takes an INJECTED
`ImportProtocolLike`. The wire-alias folding the runner's rewrite relied on
lives in `ObjectStackProtocolImplementation`; a hand-written protocol never
passes through it, so `admin-import-users.ts` read `args.query.$filter` and
got `undefined`.
The failure mode is not a missing filter but an unbounded one: `?? {}` turns
the unread key into an empty filter, the upsert duplicate probe stops
discriminating, and an admin import updates the wrong user. Both halves were
red on this branch (`admin-import-users.test.ts:560` and `:592`).
- The adapter reads `args.query.where` / `args.query.limit`, with no `??`
behind either. One dialect, and an absent `query` is a loud TypeError
rather than a silent match-everything.
- The three `import-runner` test doubles read `where` too. Two were red
(`import-runner-selfref.test.ts`, `import-runner-bulk.test.ts`); the third
was GREEN FOR THE WRONG REASON — its degraded `{}` matched the whole store,
so every no-duplicate assertion held without the probe discriminating.
- `import-runner-idempotency.test.ts` gains the assertion that closes that:
every probe must narrow, and the id recheck is pinned to the
`id: { $in: [...] }` over exactly the pre-assigned ids, bounded by `limit`.
`ImportProtocolLike.findData(args: any)` is deliberately untouched — narrowing
a published extension point is a contract decision and has its own card.
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
Co-authored-by: Claude <noreply@anthropic.com>
加宽面已交付,CI 全绿 —— ⛔ 请复审档重跑(本席不能自审自过)新 head 复审档 Finding 1 —— 已发布同胞包的回归,已关闭
复审点名的两个失败用例,现均绿:
包级读数: Finding 2 —— 三个替身,以及那条"绿得没道理",已关闭两个红替身现均绿( 复审对幂等替身提的期望是「gains an assertion that its filter actually narrowed (e.g. the recheck receives the ⭐ 但真正值得复审档读的是这一段:交付席的第一版断言没有触发。那一版只钉住 expect(appliedFilters).toEqual(calls.map(([args]) => args.query.where));替身实际施加的过滤,必须等于它被交到手里的那个 —— 一个 腿 C 还把「vacuity」这件事量化了:6 个用例里仍有 4 个通过 —— 那些 store / created / 无重复的期望,本来就是被一个什么都不区分的探测满足的。⛔ 这是测出来的,不是断言出来的。 Finding 3(MEDIUM)—— ⛔ 明确未做,且这是对的
围栏,本席在 diff 上逐条验过
另:main 自 2026-09-08 起对本 PR 的五个面(
|
维护者放宽闸门 ——
|
| CI | 39 个 check run,0 红 0 in-progress(Lint & Repo Gates 05:38:12Z 收) |
@objectstack/rest |
2 failed / 182 passed → 184 文件 / 3070 通过 / 1 skipped |
@objectstack/plugin-auth |
1 failed / 105 passed → 106 文件 / 2215 通过;复审点名的 :560 与 :592 均已转绿 |
| 围栏 | import-runner.ts 本轮零改动(⇒ findData 签名未动,留给 #16952);$filter/$top 仅 5 处且全在注释里;content/docs/releases/ 零路径 |
| 冲突源 | main 自 09-08 起对本 PR 五个面零提交 |
@objectstack/plugin-auth 定 patch 而非 minor(Check Changeset 已放行,理由是 clause-② 的契约面在 @objectstack/rest,plugin-auth 是实现方);以及上面那条 170 提交的窄化。⛔ 这两条本来是复审档要确认的,现在没有复审档了 —— 记在这里,谁日后翻到都能看见它们没被第二双眼睛过过。
合并后由本席核单亲 squash、手动收口卡 #16638(PR 用 Part of,⛔ 不会自动关),并解除 #16952 被 import-runner.ts 单写锁挡住的状态。
Generated by Claude Code
Part of #16638 — delivered as the whole atomic seam:
packages/rest's canonical QueryAST rewrite, plus the@objectstack/plugin-authadapter that rewrite would otherwise break. See "The seam, delivered whole" below. This body deliberately carries no closing keyword: whether the card's disposition changes on merge is the PM seat's call, and the durable contract fix for the class — narrowingImportProtocolLike.findData— is a separate card, #16952. #16638 stays open after this merges.Clause-②: yes
Re-declared from the DELIVERED diff, against the dispatch's
no. The path limb is untouched (packages/specis not in this diff) and no widening tell fires — no schema key, no closed-set member, no published export, no registry entry is added, and the helper's parameter is NARROWED fromany. What moved is the thing thenowas justified by: "nothing published moves" is false as delivered.ImportProtocolLikeis an exported type of the published@objectstack/rest(packages/rest/src/index.ts), itsfindData(args: any)never declared which dialect the runner sends, and this diff changes what it sends. That is measured below, not inferred — a real implementor in a sibling published package breaks.✅ The seam, delivered whole
This section previously read "⛔ Not landable alone" and listed four items as outside this PR's file surface. The PM's fence decision widened the claim to the whole atomic seam; items 1–3 are delivered here in commit
2cadb01d0e, and item 4 is filed as its own card. This branch no longer depends on anything outside itself.runImport'spis an injectedImportProtocolLike, notObjectStackProtocolImplementation. The wire-alias table lives inside that normalizer, so it folds only for callers routed through it — which is why the canonical rewrite was not, on its own, behaviour-free:runImportcallerprest-server.ts:8944—POST /data/:object/importawait this.resolveProtocol(...)rest-server.ts:9108— async import-job workerawait this.resolveProtocol(...)plugin-auth/src/admin-import-users.ts—POST /api/v1/auth/admin/import-userswhere/limit1 · The adapter — the half with a user-facing consequence ✅
The failure it removes was never a missing filter, it was an unbounded one:
args?.query?.$filter ?? {}turns an unread key into an empty filter, an empty filter constrains nothing, andfindExisting's duplicate probe stops discriminating — soPOST /api/v1/auth/admin/import-usersupdates the wrong user.content/docs/permissions/authentication.mdx:979publishes thematchBy: 'email' | 'phone'contract that falsifies.No
??behind either read. A default here would not be tolerance for an older caller — this handle is fed by the runner, never off the wire. It is precisely the lenient fallback Prime Directive #12 forbids, and it is what converts a spelling mismatch into a silent match-everything. An absentquerynow costs a loudTypeError.The two named failures this fixes, red at
03fdc6ceand green at2cadb01d:2 · A
@objectstack/plugin-authchangeset ✅.changeset/plugin-auth-admin-import-canonical-query.md, graded patch — and graded from what actually moves, not from how alarming the mechanism reads.@objectstack/plugin-authis published (files: ["dist", …]), so it owes an entry; but no published version ever shipped the mismatch. The runner's rewrite and this adapter land in the same release, and plugin-auth depends on@objectstack/restatworkspace:*, which resolves to an exact version at publish — the pair cannot be installed apart. No public API of plugin-auth changes and no behaviour of it changes against its last release, sominorwould overstate it. The sibling card #16337 graded the identical kind of rewritepatchtoo.3 · The three test doubles ✅
import-runner-selfref.test.tsargs.query?.$filter ?? {}args.query.whereimport-runner-bulk.test.tsargs.query?.$filter?.nameargs.query.where.nameimport-runner-idempotency.test.tsargs.query.where, and asserts the narrowingThe two that were red, by name:
import-runner-selfref.test.ts›resolves a row that references a record an earlier buffered row createdimport-runner-bulk.test.ts›preserves row order in results even with update/skip rows interleaved between buffered creates⛔ No dual-dialect tolerance anywhere. Not one double, and not the adapter, accepts
$filterORwhere. Every?? {}is gone: an absent filter now throws rather than degrading, in the adapter and in all three doubles alike. One dialect, canonical.4 · The idempotency assertion, and the proof that it fires ⭐
The third double passed vacuously: with
$filterundefined its filter degraded to{}, its recheck matched the entire store, and everystore/created/no-duplicateexpectation held without the probe discriminating one row from any other. A test that only passes is not the deliverable, so the file now pins both halves — the payload the probe was handed, and the filter the double actually applied:plus, on the
#3173id recheck, the exact payload:Object.keys(where)is['id'],where.id.$inequals the ids the runner pre-assigned, andlimitequals that count.Ablation — three legs, each mutation proven on disk before the run, each restored to the HEAD blob hash. The commit landed first, so every restore had a real restore point (
git checkout HEAD -- …, verified by an emptygit diff HEADand a matchinggit hash-object).$filter/$top; doubles left strict$filter ?? {}(the pre-#16638 pair):130and:169$filter ?? {}— the exact state of this branch at03fdc6ceLeg C is the load-bearing one: it is the configuration the file was actually in, and it now reds with the message that names the vacuity directly —
— while 4 of the 6 tests still pass, which is the vacuity claim measured rather than asserted: the pre-existing expectations were satisfied by a probe that discriminated nothing.
appliedFiltershalf was added for exactly that, and only then did leg C red. The intermediate result is recorded because it is the difference between an assertion that fires and one that merely exists.The fourth item is filed, not carried ⛔
ImportProtocolLike.findData(args: any)is untouched, deliberately. It is the erasure one level up and the durable fix for the class, but narrowing a published extension point is a contract decision rather than a mechanical edit — it is card #16952, not a rider here.Verification of this second commit
pnpm --filter @objectstack/rest test2 failed / 3068 passedpnpm --filter @objectstack/plugin-auth test1 file failed, 2 tests failedpnpm --filter @objectstack/rest typecheckpnpm --filter @objectstack/plugin-auth typecheckpnpm lint(whole population,eslint . --no-inline-config)scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack)check:dual-build-cjs-loads(PREREQUISITE NOT MET— nodist/) andcheck:type-check-debt(OOM at 4 GB). Both green after a fullpnpm buildand a re-run at 8 GB respectively./pulls/{n}/files. Controls fire:import-runner.tsand the pin return #16950;.changeset/returns 15 PRs. Nearest neighbours are #17195 (plugin-auth/src/identity-write-guard.ts) and #17303 (unrelatedpackages/rest/srcpins) — no overlap.origin/main, and the tool says so — 49 of the workflow/manifest files it derives from moved in that range. This branch is deliberately not merged withmain(no rebase, no force-push, and a 170-commit merge is a larger act than this fence authorises), so a family added to CI in that window would not appear in the 58. CI on the PR, and the merge queue on its rebuilt generation, are the authority there.Everything below describes the FIRST commit on this branch, fully measured.
What changed
The three literals
$filtertowhere,$toptolimit, plus theobjectthe declared query requires — the same mechanical rewrite #16337 left signposted atrest-server.ts:8972-8975.The helper — the actual deliverable
The dispatch offered
FindDataRequest['query']or dropping the helper entirely. This takes the wholeFindDataRequest, which is a strict superset of the first option: the request-levelobjectis compiled too, so theobject: ''placeholder that all three call sites had to override is gone, and each site now spells a realquery: { ... }slot — which is what lets the existing pin machinery census this file with a plumbing change rather than a second strategy.Ablation — three legs
Every mutation proven on disk by blob hash before its reading was taken; every restore by
git checkout HEAD -- ABSOLUTE_PATHunder anEXIT INT TERMtrap, verified by hash equality and an emptygit status --porcelain(which, unlikegit diff HEAD, also catches a staged index).tsc --noEmit$filter/$top, helper still typedsrc/import-runner.ts(410,47): error TS2353: Object literal may only specify known properties, and '$filter' does not exist in type 'QueryInput'.$BASE9a89a00 — three wire literals,query: anyand allOn-disk proof, leg A':
Leg C is the discriminating control the card demands: without it, leg A' only shows an error, not that this annotation is what produces it.
$BASEis the commit pinned at worktree creation, never the shared movingorigin/mainref. (A first pass at leg C mutated the signature toanyin place and exited 1 onTS6196: 'FindDataRequest' is declared but never used— an artifact of the mutation itself, with zero diagnostics on the literal. Measuring the real$BASEfile removes that ambiguity, so that is the leg reported.)No
distpreflight applies to this ablation:tsc --noEmitreadspackages/rest/srcdirectly. The preflight was used for the cross-package measurement above, which does resolve throughdist—ablation-dist-preflight.mjsconfirmed the canonical literal present indist/index.jsanddist/index.cjs, and--absentconfirmedfindArgsBase({ $filtergone from all 6 built files.Negative control — the three call paths
Added to §3 of the pin, driven through the REAL
ObjectStackProtocolImplementationnormalizer: the option bagengine.findreceives is asserted equal for the wire and canonical spelling of each of the three sites (reference resolver, duplicate probe, id recheck). All three pass, alongside §3's existing control that the instrument can tell two option bags apart.That is the control for callers that route through the normalizer. It is also exactly why the
plugin-authfinding above is a finding and not noise: the equality is a property of the normalizer, and that adapter does not use it.Pin widening
rest-server-canonical-query-ast.test.tsnow censuses the package from a table rather than one file, and the two files get different rules for a stated reason:rest-server.ts— the HTTP door. It parsesfilter/top/skip/sort/selectoff the caller's own querystring, so a wire spelling outside a server-builtquery:literal is legitimate there. Unchanged rules, floor of 5query:slots.import-runner.ts— no door; every query in it is server-built. Its census therefore rejects a wire-dialect key in object-literal position anywhere in the file, not only inside aquery:slot. Floor of 3query:slots.The whole-file rule is the one that closes the class: these three literals were arguments to a helper and were never in a
query:slot, so a slot census structurally could not have found them.Controls on the census instrument itself, because an empty result is otherwise indistinguishable from a detector that matches nothing:
{ $filter: ... }, on a key after a trailing comma across a newline, and on{ select: [] };const filter:declaration typed as a Record of string to any (a type annotation, not a key) or onwhere: filter(a value reference);The stripper drops comment-ONLY lines and keeps trailing comments — deliberately the conservative direction, so the scan can over-report loudly but never under-report silently. A string-aware tokenizer is the unsafe alternative here:
replace(/[BACKTICK-DQUOTE-SQUOTE]/g, '')inimport-runner.tsopens a quote state a simple tokenizer never closes, and everything after it would stop being scanned.§2 gains a live
@ts-expect-errorfor$filter(the alias this card retires);check:test-typecheckcompiles that layer, so an unused directive there is TS2578 — it is an assertion, not decoration. §3's picker control is now located by name rather than byPAIRS[3], since inserting rows above it would have silently re-pointed a positional reference at a different row.Verification
All at
03fdc6ceb7, working tree clean.pnpm --filter @objectstack/rest typechecktsc --noEmit+check:test-typecheck: 0 files / 0 errors)pnpm --filter @objectstack/rest testpnpm --filter '@objectstack/rest^...' buildeslint . --no-inline-config(whole repo, not narrowed)--format jsonoutput, 0 errors, 0 warnings, and both touched files are in that population. No narrowing was needed, so no invariance argument is owed; for the record the config enables no type-aware linting for any file (eslint.config.mjs:325-335).dispatch-gates.mjs --commandsthen--rancheck:dual-build-cjs-loadsandcheck:type-check-debtboth require a whole-repopnpm buildand measured nothing. Declared to CI.check:query-options-erasure(ratchet holds, baseline verified against 9a89a00, no files added),check:where-matcher,check:cross-package-test-inputs,check:test-source-alias,check:published-files,check:type-check-coverage,check:nul-bytes(8373 files, no raw control bytes)grep -naPover the three touched files: no hitsChangeset — measured, and it is required
skip-changesetis not defensible here.@objectstack/restpublishes["dist","README.md","CHANGELOG.md"], anddistmoves:ablation-dist-preflight.mjs @objectstack/rest 'query: { object: referenceObject, where: { [f]: display }, limit: 2 }'reportshit packages/rest/dist/index.cjsandhit packages/rest/dist/index.js, exit 0;--absentonfindArgsBase({ $filterreports the marker absent from all 6 built files, exit 0.So the published artifact carries the new spelling, and the payload handed to every
ImportProtocolLikeimplementor changes with it..changeset/import-runner-canonical-query-ast.mddeclares@objectstack/rest: minorand states the implementor-visible consequence explicitly. A@objectstack/plugin-authentry is owed with the adapter fix, whenever that is authorised.Docs drift — re-derived, and it is not zero
Re-derived from a clean worktree (
git status --porcelainempty at03fdc6ceb7).scripts/docs-audit/affected-docs.mjsnamed 7 pages, and printed its own coverage limit: the sdk bridge reached 60 of 216 client-bound ledger rows, so 156 are unreachable to it./:object/importbridged from the symbolrunImport— and this diff sits INSIDE that route's implementation, so the anchor catches every page about the route. "Read-only" answers whether I may edit a page, never whether the page is falsified. The discrimination, re-derived here at03fdc6ceb7(occurrence counts,importas the positive control):$filter/$topimport(control)api/client-sdk.mdxapi/wire-format.mdxdata-modeling/fields.mdxdata-modeling/import-mappings.mdxprotocol/objectql/state-machine.mdxreleases/v12.mdx(release-owned)releases/v17.mdx(release-owned)The control fires on all seven, so those six zeros are readings and not dead greps. Six pages are on the list only because they name the import route; none of them is edited.
releases/v17.mdx:3282, read out — under the heading#### Protocol & wire changes since rc.6:That sentence is a claim about what the transport ACCEPTS from a caller: it enumerates the caller-facing spellings the ingress refuses an unknown field in —
where,$filterand a raw filter AST side by side,fields/$select, and the?filter=querystring. It says nothing about what the server EMITS. This diff changes only server-built literals and leaves the door byte-identical —rest-server.tshas zero changed lines, and the whole diff is 3 files (import-runner.ts, the pin, the changeset). So the accepted wire dialect cannot have moved, and declaring those aliases at the door is #16066's half, deliberately not merged in here.⇒ Not falsified. No card filed, no edit — and it is release-owned besides, so it would not have been mine to edit either way.
Hand sweep of
content/for this change's tokens, against a live positive control (objectstack, 358 files):$filter$topfindDataQuerySchemaimport-runnerImportProtocolLikeReading: every
$filter/$toppage (odata.mdx,query-adapter.mdx,data-api.mdx,query-syntax.mdx,schema-design.mdx, ...) documents the caller-facing dialect at the HTTP door, which this change does not touch — the wire aliases stay accepted for callers, and declaring them there is #16066's spec half, deliberately not merged in here.ImportProtocolLikeappears in no page, so the extension point whose payload this diff changes is undocumented. No docs edit is owed by the in-surface diff.The sweep did catch one thing the tool's list would not have led me to:
content/docs/permissions/authentication.mdx:979documents thematchBy: 'email' | 'phone'upsert contract forPOST /api/v1/auth/admin/import-users— the exact behaviour the unmitigated change breaks. That page is not stale because of this diff; it is a published contract that item 1 above must protect.content/docs/releases/is untouched.验收备注
#16066is not merged in and nothing here declares a transport alias at the HTTP door.packages/rest/src/rest-server.tswas read (for the rest: the three server-builtfindDataliterals speak the canonical QueryAST; retirewireDialectQuery(consumer half of #16066) #16337 signpost and its landed shape) and not edited — it is a hard serial held by [finding]rest-server.ts#enforceBatchSizecalls the batch cap "deployment policy", but no shipped boot path can set it — the same defect #15543 just closed inpackages/spec, one package over #16801.ImportProtocolLike's three methods takeargs: any, so the exported extension point never declares which query dialectrunImportsends — and an in-repo implementor froze on the undeclared one #16952 (this bullet previously read "noted, not filed", handing it to the PM):ImportProtocolLike.findData(args: any),createData(args: any),updateData(args: any)are all untyped, and that erasure is why implementors froze on an undeclared dialect. Narrowing a published extension point is a contract decision, not a mechanical edit, so it stays out of this PR by design — the signature is untouched here.import-runner-idempotency.test.ts's double was green for the wrong reason on this branch. Fixed, and the vacuity is now pinned by an assertion whose firing is measured — ablation leg C above.domain:clidev seat, sessionsession_015QE8qk46e5CHJxyQEUjbf8(https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8) — recorded here in prose because an edited PR body gets a bare attribution block appended by the platform, and one block is the ruling.Generated by Claude Code