fix(rest): import-runner builds the canonical QueryAST through a typed findData envelope - #16950
fix(rest): import-runner builds the canonical QueryAST through a typed findData envelope#16950claude[bot] wants to merge 2 commits into
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 1 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 — 13 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 7b25c8f062c574b703253dd2e1a4e1ec687efac1 && git checkout 7b25c8f062c574b703253dd2e1a4e1ec687efac1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 34a76c4800d46f2aec1c136b65ff94e4087435c8 03fdc6ceb7ba691e09d9ea8a55003c35c9e01ae7 && git checkout -B drift-repro 34a76c4800d46f2aec1c136b65ff94e4087435c8 && git merge --no-ff 03fdc6ceb7ba691e09d9ea8a55003c35c9e01ae7
node scripts/docs-audit/affected-docs.mjs --json 34a76c4800d46f2aec1c136b65ff94e4087435c8
|
⛔ 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 (
|
Part of #16638 — the card's
packages/resthalf. It deliberately does not carry a closing keyword: the class this card names is not fully closed by the two files this seat was fenced to, and the remaining half is described under "Not landable alone" below. #16638 remains 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.⛔ Not landable alone
The card, triage and the dispatch all rest on one premise:
That premise holds for two of the three production callers of
runImportand fails for the third.runImport'spis an injectedImportProtocolLike, notObjectStackProtocolImplementation. The alias table lives inside the normalizer, so it folds only for callers that route through it: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:433—POST /api/v1/auth/admin/import-usersThat adapter reads the wire dialect straight off the request and calls the engine itself:
With the canonical spelling it reads
undefinedand falls back towhere: {}— an unfiltered probe. Measured, cross-package, against the rebuilt@objectstack/restdist:The engine now receives
where: {}where it receivedwhere: { phone_number: ... }. InfindExistingthat means the duplicate probe stops matching on the key: with two or more users it returnsambiguousfor every row, and with exactly one it updates the wrong user.content/docs/permissions/authentication.mdx:979states the contract this falsifies, in published docs:So the complete change is larger than the fenced file surface, and this branch must not land without it:
packages/plugins/plugin-auth/src/admin-import-users.ts:353-356— readwhere/limit. Different package, published, needs its own changeset. Out of this PR's declared file surface.packages/rest/src/import-runner-selfref.test.ts:45-46andpackages/rest/src/import-runner-bulk.test.ts:151-152— doubles that readargs.query.$filter; both are RED on this branch. Out of surface.packages/rest/src/import-runner-idempotency.test.ts:52-53— the same double. It is GREEN, and that is worse: with$filterundefined its filter degrades to{}, so it matches every row and the assertions pass for the wrong reason. Out of surface.ImportProtocolLike.findData(args: any)is itself the erasure one level up — it is why implementors froze on an undeclared dialect. Typing it is a contract decision on a published extension point, not a mechanical edit.Everything below describes what this PR does contain, 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.findData(args: any),createData(args: any),updateData(args: any)are all untyped. The successor isadmin-import-users.tsand whatever fix that adapter receives, so this is not a stray observation without a taker — but it is a contract decision on a published extension point, not a mechanical edit, and it is recorded here rather than filed because the seat's duplicate search could not be run to the standard this repo requires from inside the fence. Handing it to the PM instead.import-runner-idempotency.test.ts's double is green for the wrong reason on this branch (item 3 above). Successor: the same PR that fixes the other two doubles.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